----- "Brett Porter" <[email protected]> wrote:

> Hi Joe,
> 
> Sorry for the delayed review of this.
> 
> On 21/10/2010, at 2:02 PM, [email protected] 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?

Yes, there was a lot of duplication when I did the merge. I'm also sorting them 
out now. I was originally planning on doing another major commit but I think I 
will just commit the little fixes one by one now.
 
> > 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.

I see, I've created an issue for this for tracking purposes NPANDAY-350
 
> > +
> > +        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.

Once NPANDAY-350 is fixed then we don't need to have to create the output 
directory.
 
> > +        
> > +        
> > +        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...

Ok Noted. But wouldn't it have to be caught somewhere else? I caught it here 
thinking that way. So that the build will not break.

 
> > +    /**
> > +     * 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?

Sometimes artifact.getFile() returns a null that is why there is the need for 
the generation.

> > 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?

Ok noted. will refactor this so that it will look up in the 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?

It was including the jars and it was causing problems during compilation since 
csc/vbc aren't using them.

 
> > +        
> > +        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:\ ?

This was already fixed in NPANDAY-343

> > 
> > 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.

This was a problem during the merge.

> 
> > 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. 

Ok, but wouldn't reading from the pom take up more resources? But I agree with 
making npanday as flexible as possible.

> 
> > +           
> > +            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?

I will add it in a logger.

> > 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

Ok noted. will include this in the refactor.
 
> > 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?

This was already failing for me before I added in any changes for the pab & uac 
removal
 
> > 
> > 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)

The problem was it used the old compilers so we need a way to have the 
unreleased compilers used. Because if we use the release once. It will still 
have the pab and uac removal. But I think just for release purposes we can use 
the old compilers to release the new one. and Then after the release we can go 
back one version earlier.
 
> - Brett
> 
> --
> Brett Porter
> [email protected]
> http://brettporter.wordpress.com/

Reply via email to