You are viewing a plain text version of this content. The canonical link for it is here.
Posted to npanday-dev@incubator.apache.org by Brett Porter <br...@apache.org> on 2010/10/26 23:38:47 UTC

Re: svn commit: r1025815 - in /incubator/npanday/trunk: ./ components/dotnet-artifact/src/main/java/npanday/artifact/impl/ components/dotnet-core/ components/dotnet-core/src/main/java/npanday/ components/dotnet-dao-project/src/main/java/npanday/dao/ compon...

Hi Joe,

Sorry for the delayed review of this.

On 21/10/2010, at 2:02 PM, jocaba@apache.org wrote:

> Modified: incubator/npanday/trunk/components/dotnet-artifact/src/main/java/npanday/artifact/impl/ArtifactInstallerImpl.java
> URL: http://svn.apache.org/viewvc/incubator/npanday/trunk/components/dotnet-artifact/src/main/java/npanday/artifact/impl/ArtifactInstallerImpl.java?rev=1025815&r1=1025814&r2=1025815&view=diff
> ==============================================================================
> --- incubator/npanday/trunk/components/dotnet-artifact/src/main/java/npanday/artifact/impl/ArtifactInstallerImpl.java (original)
> +++ incubator/npanday/trunk/components/dotnet-artifact/src/main/java/npanday/artifact/impl/ArtifactInstallerImpl.java Thu Oct 21 03:02:36 2010
> @@ -296,6 +297,7 @@ public class ArtifactInstallerImpl
>             }
>             artifactDependencies.add( artifact );
>         }
> +        //File installDirectory = PathUtil.getPrivateApplicationBaseFileFor( artifact, localRepository ).getParentFile();

This seems duplicated?

> Modified: incubator/npanday/trunk/components/dotnet-core/src/main/java/npanday/PathUtil.java
> URL: http://svn.apache.org/viewvc/incubator/npanday/trunk/components/dotnet-core/src/main/java/npanday/PathUtil.java?rev=1025815&r1=1025814&r2=1025815&view=diff
> ==============================================================================
> --- incubator/npanday/trunk/components/dotnet-core/src/main/java/npanday/PathUtil.java (original)
> +++ incubator/npanday/trunk/components/dotnet-core/src/main/java/npanday/PathUtil.java Thu Oct 21 03:02:36 2010
> @@ -25,9 +25,21 @@ import npanday.ArtifactType;
> import org.codehaus.plexus.util.DirectoryScanner;
> import org.codehaus.plexus.util.FileUtils;
> 
> +import org.w3c.dom.Document;
> +import org.w3c.dom.Element;
> +import org.w3c.dom.Node;
> +import org.w3c.dom.NodeList;

I think these were unintentional.

> +        String outputDir = System.getProperty("user.dir");
> +        outputDir = outputDir+File.separator+"target";

(within getDotNetArtifact method)

This is not a good approach to getting the path. In a reactor build, user.dir will differ depending on where you run the build (top-level vs module). Accessing system properties is a problem for embedding in other applications as well. Finally, the project can redefine the target directory so it shouldn't be hardcoded.

I'd recommend passing the location in - it'll make you backtrack to the mojos, where you have access to ${project.build.directory} which is always correctly set. In some cases, you may actually want to use a different directory anyway, so it's good to have the option.

> +
> +        new File(outputDir).mkdir();

I don't think this is this method's responsibility - you probably needed it for multi-module builds using the wrong directory

> +           
> +        String filename = artifact.getArtifactId() + "." + artifact.getArtifactHandler().getExtension();
> +        File targetFile = new File(outputDir+File.separator+ filename);

Note that you don't need to use File.separator all the time. For plain string paths, you can just use / as Java translates it (eg, outputDir += "/target" above, if that code was still to be used). In File, you can actually use the constructor new File( outputDir, filename ) as well.

> +        
> +        
> +        try
> +        {    
> +              FileUtils.copyFile(new File( source ), targetFile);
> +        }   
> +        catch (IOException ioe) 
> +        {
> +            logger.warning("\nNPANDAY-1005-0001: Error copying dependency " + artifact +" "+ioe.getMessage());
> +        }

Should this only be a warning? Won't it cause problems later? I'd think it should be thrown...
> 
> +    /**
> +     * Returns the path of the artifact within the user assembly cache.
> +     *
> +     * @param artifact        the artifact to find the path of. This value should not be null.
> +     * @return the path of the artifact within the user assembly cache or null if either of the specified
> +     *         parameters is null
> +     */
> +    public static File getDotNetArtifact( Artifact artifact, File localRepository )
> +    {
> +        if ( artifact == null )
> +        {
> +            logger.warning( "NPANDAY-040-0532: Artifact is null - Cannot get application file." );
> +            return null;
> +        }
> +       
> +        String ext = ArtifactType.getArtifactTypeForPackagingName( artifact.getType() ).getExtension();
> +        
> +        //assumes that since it was not found as a .dll or a .exe it will be considered as a default library
> +        if(ext == null)
> +        {
> +            ext = "jar";
> +        }

I don't quite understand this - why would the type not be found? (it may not be important - see below)

> +        
> +        File source = null;
> +        
> +        String classifier = "";
> +        
> +        if(artifact.getClassifier()!= null)
> +        {
> +            classifier = "-"+artifact.getClassifier();
> +        }        
> +           
> +        
> +        if( localRepository!= null )
> +        {
> +          source = new File( localRepository + File.separator + getTokenizedPath(artifact.getGroupId() ) + File.separator + artifact.getArtifactId() + File.separator + artifact.getVersion() + File.separator + artifact.getArtifactId() + "-" + artifact.getVersion() + classifier +"." + ext );
> +        }
> +        else
> +        {
> +           source = new File( System.getProperty( "user.home" ),".m2" + File.separator + "repository" + File.separator + getTokenizedPath(artifact.getGroupId() ) + File.separator + artifact.getArtifactId() + File.separator + artifact.getVersion() + File.separator + artifact.getArtifactId() + "-" + artifact.getVersion() +"." + ext );
> +        
> +        }
> +                      
> +        File dotnetFile =  getDotNetArtifact( artifact, source.toString() );
> +        
> +        return dotnetFile;
>     }

You shouldn't construct the path yourself. I'm not sure where this is used, but it should be passed from the result of artifact resolution. Is artifact.getFile() already set?

> Modified: incubator/npanday/trunk/components/dotnet-dao-project/src/main/java/npanday/dao/ProjectFactory.java
> URL: http://svn.apache.org/viewvc/incubator/npanday/trunk/components/dotnet-dao-project/src/main/java/npanday/dao/ProjectFactory.java?rev=1025815&r1=1025814&r2=1025815&view=diff
> ==============================================================================
> --- incubator/npanday/trunk/components/dotnet-dao-project/src/main/java/npanday/dao/ProjectFactory.java (original)
> +++ incubator/npanday/trunk/components/dotnet-dao-project/src/main/java/npanday/dao/ProjectFactory.java Thu Oct 21 03:02:36 2010
> @@ -20,6 +20,7 @@ package npanday.dao;
> 
> import npanday.ArtifactType;
> import npanday.ArtifactTypeHelper;
> +import npanday.PathUtil;
> import org.apache.maven.artifact.Artifact;
> import org.apache.maven.artifact.factory.ArtifactFactory;
> import org.apache.maven.model.Dependency;
> @@ -211,14 +212,12 @@ public final class ProjectFactory
>                                                                           project.getArtifactType(),
>                                                                           project.getPublicKeyTokenId() );
> 
> +                
>         File artifactFile = ( ( ArtifactTypeHelper.isDotnetAnyGac( project.getArtifactType() ) ) ) ? new File(
>             "C:\\WINDOWS\\assembly\\" + project.getArtifactType() + File.separator + project.getArtifactId() + File.separator +
> -                project.getVersion() + "__" + project.getPublicKeyTokenId() + File.separator + project.getArtifactId() + ".dll" )
> -            : new File( localRepository.getParentFile(), File.separator + "uac" + File.separator + "gac_msil" + File.separator
> -                + project.getArtifactId() + File.separator +
> -                project.getVersion() + "__" + project.getGroupId() + File.separator + project.getArtifactId() + "." +
> -                ArtifactType.getArtifactTypeForPackagingName( project.getArtifactType() ).getExtension() );
> -
> +                project.getVersion() + "__" + project.getPublicKeyTokenId() + File.separator + project.getArtifactId() + ArtifactType.getArtifactTypeForPackagingName( project.getArtifactType() ).getExtension()  )
> +            : PathUtil.getDotNetArtifact( assembly, localRepository ) ;
> +        

the GAC construction happens in multiple places as well (eg ProjectDaoImpl), should that be moved to PathUtil?

> 
> Modified: incubator/npanday/trunk/components/dotnet-dao-project/src/main/java/npanday/dao/impl/ProjectDaoImpl.java
> URL: http://svn.apache.org/viewvc/incubator/npanday/trunk/components/dotnet-dao-project/src/main/java/npanday/dao/impl/ProjectDaoImpl.java?rev=1025815&r1=1025814&r2=1025815&view=diff
> ==============================================================================
> --- incubator/npanday/trunk/components/dotnet-dao-project/src/main/java/npanday/dao/impl/ProjectDaoImpl.java (original)
> +++ incubator/npanday/trunk/components/dotnet-dao-project/src/main/java/npanday/dao/impl/ProjectDaoImpl.java Thu Oct 21 03:02:36 2010
> @@ -294,12 +294,16 @@ public final class ProjectDaoImpl
> 
>             if ( !result.hasNext() )
>             {
> -                if ( artifactType != null && ArtifactTypeHelper.isDotnetAnyGac( artifactType ) )
> +                
> +                //if ( artifactType != null && ArtifactTypeHelper.isDotnetAnyGac( artifactType ) )
> +                if ( artifactType != null )

why was this changed?

> Modified: incubator/npanday/trunk/components/dotnet-executable/src/main/java/npanday/executable/compiler/impl/DefaultCompiler.java
> URL: http://svn.apache.org/viewvc/incubator/npanday/trunk/components/dotnet-executable/src/main/java/npanday/executable/compiler/impl/DefaultCompiler.java?rev=1025815&r1=1025814&r2=1025815&view=diff
> ==============================================================================
> --- incubator/npanday/trunk/components/dotnet-executable/src/main/java/npanday/executable/compiler/impl/DefaultCompiler.java (original)
> +++ incubator/npanday/trunk/components/dotnet-executable/src/main/java/npanday/executable/compiler/impl/DefaultCompiler.java Thu Oct 21 03:02:36 2010
> @@ -108,7 +108,11 @@ public final class DefaultCompiler
>             for ( Artifact artifact : references )
>             {
>                 String path = artifact.getFile().getAbsolutePath();
> -                commands.add( "/reference:" + path );
> +          
> +                if( !path.contains( ".jar" ) )
> +                {
> +                    commands.add( "/reference:" + path );
> +                }

This hardcoding doesn't seem good - it seems linked to the other change above about unknown extensions. Shouldn't it be checking the artifact types against suitable references?
> 
> +        
> +        try
> +        {
> +             FileUtils.copyFile(new File(responseFilePath),new File( "c:/responsefile.txt"));
> +        }
> +        catch (java.io.IOException e) {
> +    		throw new ExecutionException( "Error while creating response file for the commands.", e );
> +        }
> +       
> +        

Why a hardcoded copy to c:\ ?

> 
> Modified: incubator/npanday/trunk/components/dotnet-executable/src/main/java/npanday/executable/impl/CompilerContextImpl.java
> URL: http://svn.apache.org/viewvc/incubator/npanday/trunk/components/dotnet-executable/src/main/java/npanday/executable/impl/CompilerContextImpl.java?rev=1025815&r1=1025814&r2=1025815&view=diff
> ==============================================================================
> --- incubator/npanday/trunk/components/dotnet-executable/src/main/java/npanday/executable/impl/CompilerContextImpl.java (original)
> +++ incubator/npanday/trunk/components/dotnet-executable/src/main/java/npanday/executable/impl/CompilerContextImpl.java Thu Oct 21 03:02:36 2010
> @@ -28,6 +28,7 @@ import npanday.executable.compiler.*;
> import npanday.artifact.ArtifactContext;
> import npanday.artifact.ArtifactException;
> import npanday.ArtifactType;
> +import npanday.PathUtil;
> 
> import org.apache.maven.project.MavenProject;
> import org.apache.maven.artifact.Artifact;
> @@ -388,7 +389,19 @@ public final class CompilerContextImpl
>                 logger.debug( "NPANDAY-061-006: Artifact Type:" + type);
>                 logger.debug( "NPANDAY-061-007: Artifact Type:" + ArtifactTypeHelper.isDotnetGenericGac( type ));
>                 ArtifactType artifactType = ArtifactType.getArtifactTypeForPackagingName( type );
> -
> +                if ( ArtifactTypeHelper.isDotnetModule( type ))
> +                {
> +                    modules.add( artifact );
> +                }
> +                else if ( (artifactType != ArtifactType.NULL && (
> +                            StringUtils.equals( artifactType.getTargetCompileType(), "library" )
> +                            || artifactType.getExtension().equals( "dll" )
> +                            || artifactType.getExtension().equals( "exe" ))
> +                          )
> +                          || type.equals( "jar" ) )
> +                {
> +                    libraries.add( artifact );
> +                }

Is there an isDotnet* method that better represents this? Why .jar?

> Modified: incubator/npanday/trunk/components/dotnet-registry/src/main/java/npanday/registry/ConnectionsRepository.java
> URL: http://svn.apache.org/viewvc/incubator/npanday/trunk/components/dotnet-registry/src/main/java/npanday/registry/ConnectionsRepository.java?rev=1025815&r1=1025814&r2=1025815&view=diff
> ==============================================================================
> --- incubator/npanday/trunk/components/dotnet-registry/src/main/java/npanday/registry/ConnectionsRepository.java (original)
> +++ incubator/npanday/trunk/components/dotnet-registry/src/main/java/npanday/registry/ConnectionsRepository.java Thu Oct 21 03:02:36 2010
> @@ -55,9 +62,9 @@ public class ConnectionsRepository
>      */
>     public void lazyLoad() throws IOException
>     {
> -        long start = System.currentTimeMillis();
> +       long start = System.currentTimeMillis();
> 
> -        File dataDir = new File( System.getProperty( "user.home" ), ".m2/uac/rdfRepository" );
> +        File dataDir = new File( System.getProperty( "user.home" ), ".m2/repository" );

We should avoid this hardcoding - but if the RDF repository is being removed it won't matter :)

> Modified: incubator/npanday/trunk/components/dotnet-repository/src/test/java/npanday/repository/impl/RepositoryConverterImplTest.java
> URL: http://svn.apache.org/viewvc/incubator/npanday/trunk/components/dotnet-repository/src/test/java/npanday/repository/impl/RepositoryConverterImplTest.java?rev=1025815&r1=1025814&r2=1025815&view=diff
> ==============================================================================
> --- incubator/npanday/trunk/components/dotnet-repository/src/test/java/npanday/repository/impl/RepositoryConverterImplTest.java (original)
> +++ incubator/npanday/trunk/components/dotnet-repository/src/test/java/npanday/repository/impl/RepositoryConverterImplTest.java Thu Oct 21 03:02:36 2010
> @@ -30,64 +30,7 @@ public class RepositoryConverterImplTest
> 
>     private static File basedir = new File( System.getProperty( "basedir" ) );
> 
> -
> -    public void testConvertArtifact()
> -    {
> -        File testRepo = new File( System.getProperty( "basedir" ), "target/test-repo/repository-1" );
> -        testRepo.mkdir();
> -
> -        Repository repository = this.createRepository();
> -        ProjectDao dao = this.createProjectDao( repository );
> -
> -        Project project = new Project();
> -        project.setGroupId( "npanday.model" );
> -        project.setArtifactId( "NPanday.Model.Pom" );
> -        project.setVersion( "1.0" );
> -        project.setArtifactType( "library" );
> -        project.setPublicKeyTokenId( "abc" );
> -
> -        ProjectDependency test2 = createProjectDependency( "npanday", "NPanday.Test", "1.0" );
> -        test2.setArtifactType( "library" );
> -        project.addProjectDependency( test2 );
> -
> -        try
> -        {
> -            dao.storeProjectAndResolveDependencies( project, testRepo, new ArrayList<ArtifactRepository>() );
> -        }
> -        catch ( java.io.IOException e )
> -        {
> -            e.printStackTrace();
> -            fail( "Could not store the project: " + e.getMessage() );
> -        }
> -
> -        RepositoryConverterImpl repositoryConverter = new RepositoryConverterImpl();
> -        repositoryConverter.initTest( new DataAccessObjectRegistryStub(), new ArtifactFactoryTestStub(),
> -                                      null, new ArtifactResolverTestStub() );
> -
> -        ArtifactFactory artifactFactory = new ArtifactFactoryTestStub();
> -        Artifact artifact = artifactFactory.createArtifactWithClassifier( project.getGroupId(), project.getArtifactId(),
> -                                                                          project.getVersion(),
> -                                                                          project.getArtifactType(), "abc" );
> -        File artifactFile = new File( testRepo.getParentFile(),
> -                                      "/uac/gac_msil/NPanday.Model.Pom/1.0__npanday.model/NPanday.Model.Pom.dll" );
> -
> -        artifact.setFile( artifactFile );
> -        try
> -        {
> -            repositoryConverter.convertRepositoryFormatFor( artifact, null, repository, testRepo );
> -        }
> -        catch ( IOException e )
> -        {
> -            fail( "Could not convert the repository: " + e.getMessage() );
> -        }
> -        this.exportRepositoryToRdf( "testConvertArtifact-rdf.xml", testRepo, repository );
> -
> -        assertTrue( new File( testRepo, "/npanday/model/NPanday.Model.Pom/1.0/NPanday.Model.Pom-1.0-abc.dll" ).exists() );
> -        assertTrue( new File( testRepo, "/npanday/model/NPanday.Model.Pom/1.0/NPanday.Model.Pom-1.0.pom" ).exists() );
> -        assertFalse( new File( testRepo, "/npanday/NPanday.Test/1.0/NPanday.Test-1.0.dll" ).exists() );
> -        assertFalse( new File( testRepo, "/npanday/NPanday.Test/1.0/NPanday.Test-1.0.pom" ).exists() );
> -    }

Why was this removed?

> -
> +    
>     public void testConvert()
>     {
>         File testRepo = new File( System.getProperty( "basedir" ), "target/test-repo/repository" );
> @@ -101,9 +44,8 @@ public class RepositoryConverterImplTest
>         project.setArtifactId( "NPanday.Model.Pom" );
>         project.setVersion( "1.0" );
>         project.setArtifactType( "library" );
> -        project.setPublicKeyTokenId( "abc" );
> 
> -        ProjectDependency test2 = createProjectDependency( "npanday", "NPanday.Test", "1.0" );
> +        ProjectDependency test2 = createProjectDependency( "npanday", "ClassLibrary1", "1.0" );
>         test2.setArtifactType( "library" );
>         project.addProjectDependency( test2 );
> 
> @@ -130,10 +72,10 @@ public class RepositoryConverterImplTest
>         }
>         this.exportRepositoryToRdf( "testConvert-rdf.xml", testRepo, repository );
> 
> -        assertTrue( new File( testRepo, "/npanday/model/NPanday.Model.Pom/1.0/NPanday.Model.Pom-1.0-abc.dll" ).exists() );
> +        assertTrue( new File( testRepo, "/npanday/model/NPanday.Model.Pom/1.0/NPanday.Model.Pom-1.0.dll" ).exists() );
>         assertTrue( new File( testRepo, "/npanday/model/NPanday.Model.Pom/1.0/NPanday.Model.Pom-1.0.pom" ).exists() );
> -        assertTrue( new File( testRepo, "/npanday/NPanday.Test/1.0/NPanday.Test-1.0.dll" ).exists() );
> -        assertTrue( new File( testRepo, "/npanday/NPanday.Test/1.0/NPanday.Test-1.0.pom" ).exists() );
> +        assertTrue( new File( testRepo, "/npanday/ClassLibrary1/1.0/ClassLibrary1-1.0.dll" ).exists() );
> +

Are these results correct? I realise you needed to remove the UAC, but not sure why the test for the model (particularly the classifier) was removed.

> Modified: incubator/npanday/trunk/dotnet/assemblies/NPanday.Artifact/src/main/csharp/NPanday/Artifact/ArtifactRepository.cs
> URL: http://svn.apache.org/viewvc/incubator/npanday/trunk/dotnet/assemblies/NPanday.Artifact/src/main/csharp/NPanday/Artifact/ArtifactRepository.cs?rev=1025815&r1=1025814&r2=1025815&view=diff
> ==============================================================================
> --- incubator/npanday/trunk/dotnet/assemblies/NPanday.Artifact/src/main/csharp/NPanday/Artifact/ArtifactRepository.cs (original)
> +++ incubator/npanday/trunk/dotnet/assemblies/NPanday.Artifact/src/main/csharp/NPanday/Artifact/ArtifactRepository.cs Thu Oct 21 03:02:36 2010
> @@ -26,6 +26,7 @@ using System.Text;
> using System.Xml;
> using System.Xml.Serialization;
> using System.Windows.Forms;
> +using NPanday.Model.Setting;
> 
> namespace NPanday.Artifact
> {
> @@ -34,8 +35,14 @@ namespace NPanday.Artifact
> 
>         public string GetLocalUacPath(Artifact artifact, string ext)
>         {
> -            return Path.Combine(localRepository.FullName, string.Format(@"uac\gac_msil\{1}\{2}__{0}\{1}{3}", artifact.GroupId, artifact.ArtifactId, artifact.Version, ext));
> +            return Path.Combine(SettingsUtil.GetLocalRepositoryPath(), string.Format(@"{0}\{1}\{1}{2}-{3}", Tokenize(artifact.GroupId), artifact.ArtifactId, artifact.Version, 

This is duplicated with PathUtil.cs. They should be shared. Also, does the Artifact type support classifiers? They don't seem to be included here.

> @@ -56,7 +63,7 @@ namespace NPanday.Artifact
>         {
>             Artifact artifact = new Artifact();
> 
> -            DirectoryInfo uac = new DirectoryInfo(localRepository.FullName + @"\uac\gac_msil\");
> +            DirectoryInfo uac = new DirectoryInfo(localRepository.FullName);
> 
>             String[] tokens = uri.Split("/".ToCharArray(), StringSplitOptions.RemoveEmptyEntries);
>             int size = tokens.Length;
> @@ -93,8 +100,8 @@ namespace NPanday.Artifact
>                 artifact.Extension = extToken[extToken.Length - 1];
>             }
> 
> -            artifact.FileInfo = new FileInfo(uac.FullName + artifact.ArtifactId + @"\" +
> -                    artifact.Version + "__" + artifact.GroupId + @"\" + artifact.ArtifactId + ".dll");
> +            artifact.FileInfo = new FileInfo(uac.FullName + Tokenize( artifact.GroupId )+ Path.DirectorySeparatorChar + artifact.ArtifactId + Path.DirectorySeparatorChar 
> +                + artifact.Version + Path.DirectorySeparatorChar + artifact.ArtifactId+ "-" + artifact.Version+ ".dll");

Seems like a dupe of above too.

> Modified: incubator/npanday/trunk/dotnet/assemblies/NPanday.Artifact/src/main/csharp/NPanday/Artifact/PathUtil.cs
> URL: http://svn.apache.org/viewvc/incubator/npanday/trunk/dotnet/assemblies/NPanday.Artifact/src/main/csharp/NPanday/Artifact/PathUtil.cs?rev=1025815&r1=1025814&r2=1025815&view=diff
> ==============================================================================
> --- incubator/npanday/trunk/dotnet/assemblies/NPanday.Artifact/src/main/csharp/NPanday/Artifact/PathUtil.cs (original)
> +++ incubator/npanday/trunk/dotnet/assemblies/NPanday.Artifact/src/main/csharp/NPanday/Artifact/PathUtil.cs Thu Oct 21 03:02:36 2010
> @@ -23,21 +23,35 @@ using System;
> using System.Collections.Generic;
> using System.IO;
> using System.Text;
> +using NPanday.Model.Setting;
> 
> namespace NPanday.Artifact
> {
>     public class PathUtil
>     {
> -        public static FileInfo GetPrivateApplicationBaseFileFor(Artifact artifact, DirectoryInfo localRepository)
> +        public static FileInfo GetPrivateApplicationBaseFileFor(Artifact artifact, DirectoryInfo localRepository, string currentDir)
>         {
> -            return new FileInfo(localRepository.Parent.FullName + @"\pab\gac_msil\" + artifact.ArtifactId + @"\" + artifact.Version + "__" +
> -                artifact.GroupId + @"\" + artifact.ArtifactId + "." + artifact.Extension);
> +            FileInfo target = new FileInfo(currentDir + Path.PathSeparator + "target" + Path.PathSeparator+artifact.ArtifactId + artifact.Extension);

This is probably ok, but if there's a way to read it from the POM or pass in the target directory rather than just the basedir, that'd be more flexible. 

> +           
> +            FileInfo source = GetUserAssemblyCacheFileFor(artifact, localRepository);
> +
> +            if(source.Exists)
> +            {
> +                   File.Copy( source.ToString(), target.ToString());
> +            } 
> +            
> +           

Why might these be different?
> 
> Modified: incubator/npanday/trunk/dotnet/assemblies/NPanday.Plugin.MojoGenerator/src/main/csharp/NPanday/Plugin/MojoGenerator/Generator.cs
> URL: http://svn.apache.org/viewvc/incubator/npanday/trunk/dotnet/assemblies/NPanday.Plugin.MojoGenerator/src/main/csharp/NPanday/Plugin/MojoGenerator/Generator.cs?rev=1025815&r1=1025814&r2=1025815&view=diff
> ==============================================================================
> --- incubator/npanday/trunk/dotnet/assemblies/NPanday.Plugin.MojoGenerator/src/main/csharp/NPanday/Plugin/MojoGenerator/Generator.cs (original)
> +++ incubator/npanday/trunk/dotnet/assemblies/NPanday.Plugin.MojoGenerator/src/main/csharp/NPanday/Plugin/MojoGenerator/Generator.cs Thu Oct 21 03:02:36 2010
> @@ -83,7 +83,7 @@ namespace NPanday.Plugin.MojoGenerator
> 			char[] delim = {'.'};
> 			DirectoryInfo sourceDirectory = new DirectoryInfo(@outputDirectory.FullName + "/src/main/java/" 
> 			                                                  + artifactId.Replace('.', '/'));
> -			sourceDirectory.Create();
> +            sourceDirectory.Create();
> 			if(javaClasses.Count == 0)
> 			{
> 				Console.WriteLine("NPanday-000-000: There are no Mojos within the assembly: Artifact Id = " 
> @@ -97,9 +97,11 @@ namespace NPanday.Plugin.MojoGenerator
> 				string classFileName = tokens[tokens.Length - 1];
> 				FileInfo fileInfo = new FileInfo(sourceDirectory.FullName + "/" 
> 				                                 + classFileName + ".java");
> -				jcuLocal.unmarshall(javaClass, fileInfo);
> +                jcuLocal.unmarshall(javaClass, fileInfo);
> 			}
> -			
> +            try
> +            {		    
> +
>             TextReader reader = new StreamReader(Assembly.GetExecutingAssembly().
>             GetManifestResourceStream(Assembly.GetExecutingAssembly().GetManifestResourceNames()[0]));
> 			XmlSerializer serializer = new XmlSerializer(typeof(NPanday.Model.Pom.Model));
> @@ -108,10 +110,15 @@ namespace NPanday.Plugin.MojoGenerator
> 			model.groupId = groupId;
> 			model.version = version;
> 			model.name = artifactId + ".JavaBinding";
> -				    
> -			FileInfo outputPomXml = new FileInfo(@outputDirectory.FullName + "/pom-java.xml");
> +
> +            FileInfo outputPomXml = new FileInfo(@outputDirectory.FullName + "/pom-java.xml");
> 			TextWriter textWriter = new StreamWriter(@outputPomXml.FullName);
> -			serializer.Serialize(textWriter, model);
> +            serializer.Serialize(textWriter, model);
> +            }
> +            catch (Exception e)
> +            {
> +                Console.WriteLine(e);

Is swallowing the exception a good idea?
> 
> Modified: incubator/npanday/trunk/dotnet/assemblies/NPanday.ProjectImporter/Engine/src/main/csharp/NPanday/ProjectImporter/Digest/Model/Reference.cs
> URL: http://svn.apache.org/viewvc/incubator/npanday/trunk/dotnet/assemblies/NPanday.ProjectImporter/Engine/src/main/csharp/NPanday/ProjectImporter/Digest/Model/Reference.cs?rev=1025815&r1=1025814&r2=1025815&view=diff
> ==============================================================================
> --- incubator/npanday/trunk/dotnet/assemblies/NPanday.ProjectImporter/Engine/src/main/csharp/NPanday/ProjectImporter/Digest/Model/Reference.cs (original)
> +++ incubator/npanday/trunk/dotnet/assemblies/NPanday.ProjectImporter/Engine/src/main/csharp/NPanday/ProjectImporter/Digest/Model/Reference.cs Thu Oct 21 03:02:36 2010
> @@ -473,7 +473,12 @@ namespace NPanday.ProjectImporter.Digest
> 
>         public static string GetLocalUacPath(Artifact.Artifact artifact, string ext)
>         {
> -            return Path.Combine(Directory.GetParent(SettingsUtil.GetLocalRepositoryPath()).FullName, string.Format(@"uac\gac_msil\{1}\{2}__{0}\{1}{3}", artifact.GroupId, artifact.ArtifactId, artifact.Version, ext));
> +            return Path.Combine(SettingsUtil.GetLocalRepositoryPath(), string.Format(@"{0}\{1}\{1}{2}-{3}", Tokenize(artifact.GroupId), artifact.ArtifactId, artifact.Version, ext));
> +        }

This should also reuse the PathUtil
> 
> Modified: incubator/npanday/trunk/dotnet/pom.xml
> URL: http://svn.apache.org/viewvc/incubator/npanday/trunk/dotnet/pom.xml?rev=1025815&r1=1025814&r2=1025815&view=diff
> ==============================================================================
> --- incubator/npanday/trunk/dotnet/pom.xml (original)
> +++ incubator/npanday/trunk/dotnet/pom.xml Thu Oct 21 03:02:36 2010
> @@ -35,7 +35,8 @@ under the License.
>   </modules>  
>   <build> 
>     <sourceDirectory>src/main/csharp</sourceDirectory>  
> -    <testSourceDirectory>src/test/csharp</testSourceDirectory>  
> +    <!-- Disabled because of NUnit-console Issue NPANDAY-332 -->
> +    <!-- <testSourceDirectory>src/test/csharp</testSourceDirectory> -->

Did this work for you before you applied your changes? Or could it be related to the VS2010 changes?

> 
> Modified: incubator/npanday/trunk/plugins/maven-repository-plugin/src/main/java/npanday/plugin/repository/RepositoryRdfExporterMojo.java
> URL: http://svn.apache.org/viewvc/incubator/npanday/trunk/plugins/maven-repository-plugin/src/main/java/npanday/plugin/repository/RepositoryRdfExporterMojo.java?rev=1025815&r1=1025814&r2=1025815&view=diff
> ==============================================================================
> --- incubator/npanday/trunk/plugins/maven-repository-plugin/src/main/java/npanday/plugin/repository/RepositoryRdfExporterMojo.java (original)
> +++ incubator/npanday/trunk/plugins/maven-repository-plugin/src/main/java/npanday/plugin/repository/RepositoryRdfExporterMojo.java Thu Oct 21 03:02:36 2010
> @@ -67,8 +71,7 @@ public class RepositoryRdfExporterMojo
>         RDFHandler rdfxmlWriter;
>         try
>         {
> -            File exportFile = new File( localRepository.getParentFile(), "/uac/rdfRepository/rdf-repository-export.xml" );
> -            rdfxmlWriter = new RDFXMLWriter( new FileOutputStream( exportFile ) );
> +            rdfxmlWriter = new RDFXMLWriter( new FileOutputStream( localRepository ) );

This seems to have dropped the filename.

> @@ -478,7 +478,8 @@ under the License.
>     <mavenVersion>2.0.9</mavenVersion>  
>     <npanday.snapshots.url>http://repo.npanday.org/archiva/repository/npanday-snapshots</npanday.snapshots.url>
>     <npanday.releases.url>http://repo.npanday.org/archiva/repository/npanday-releases</npanday.releases.url>
> -    <stable.npanday.version>1.2</stable.npanday.version>
> +    <!--<stable.npanday.version>1.2</stable.npanday.version>-->
> +     <stable.npanday.version>1.2.2-incubating-SNAPSHOT</stable.npanday.version>

This will make it hard to release. I noticed your comment when resolving the issue about this. What problem occurs if you use the released version?

(Note that old versions should be able to coincide with new ones)

- Brett

--
Brett Porter
brett@apache.org
http://brettporter.wordpress.com/


Re: svn commit: r1025815 - in /incubator/npanday/trunk: ./ components/dotnet-artifact/src/main/java/npanday/artifact/impl/ components/dotnet-core/ components/dotnet-core/src/main/java/npanday/ components/dotnet-dao-project/src/main/java/npanday/dao/ compon...

Posted by Lars Corneliussen <me...@lcorneliussen.de>.
Couldn't agree more.

All path construction should be done in PathUtil.cs and PathUtil.java 
which even should be exact copies.
Also path construction is real code and should be structured in a clean 
way without any duplication.

Am 27.10.10 01:38, schrieb Brett Porter:
> +           source = new File( System.getProperty( "user.home" ),".m2" + File.separator + "repository" + File.separator + getTokenizedPath(artifact.getGroupId() ) + File.separator + artifact.getArtifactId() + File.separator + artifact.getVersion() + File.separator + artifact.getArtifactId() + "-" + artifact.getVersion() +"." + ext );