NPanday
  1. NPanday
  2. NPANDAY-322

Resync Reference doesn't update SNAPSHOT artifact from local repository that already exist in .references folder

    Details

    • Codeplex ID:
      14093

      Description

      Steps:
      1. Install Library1 into local maven repo
      2. Add Library1 as dependency using Resync Reference to ProjectA (it will be copied into .references folder)
      3. Update and reinstall Library1 into local maven repo
      4. Invoke Resync Reference for ProjectA
      5. Error: Library1 won't be updated in .references folder

      Expected: newer version (in terms of file timestamp) of Library1 (if any) should be copied into .references folder from local maven repo during Resync Reference

      Issue exist in trunk r59731

      1. NPANDAY-322_and_NPANDAY-476.diff
        25 kB
        Stoyan Damov
      2. ReferenceManager.cs
        11 kB
        Stoyan Damov
      3. ArtifactUtils.cs
        5 kB
        Stoyan Damov
      4. Connect.cs
        104 kB
        Stoyan Damov
      5. npanday.its.IntegrationTestSuite.txt
        21 kB
        Stoyan Damov
      6. TEST-npanday.its.IntegrationTestSuite.xml
        45 kB
        Stoyan Damov

        Activity

        Hide
        Lars Corneliussen added a comment -

        Hi. My fault. I have no profile in my setting, which makes NPanday complain. I created NPANDAY-478 and fixed it locally.

        Find my on Skype "lcorneliussen" or on webchat.freenode.net in the #npanday channel

        Show
        Lars Corneliussen added a comment - Hi. My fault. I have no profile in my setting, which makes NPanday complain. I created NPANDAY-478 and fixed it locally. Find my on Skype "lcorneliussen" or on webchat.freenode.net in the #npanday channel
        Hide
        Stoyan Damov added a comment - - edited

        I'll try to make it clearer:

        First, here's the new code:

        if (!ArtifactUtils.Exists(artifact) || (isSnapshot && resyncFromRemoteRepo))
        {
            if (!ArtifactUtils.DownloadFromRemoteRepository(artifact, logger))
            {
                RaiseError("Unable to get the artifact {0} from any of your repositories.", artifact.ArtifactId);
                return;
            }
        }
        
        CopyToReferenceFolder(artifact, referenceFolder);
        

        TODO: gotta go, will comment a bit later tonight but I see a flaw already, the code should read:

        bool artifactExists = ArtifactUtils.Exists(artifact);
        if (!artifactExists || (isSnapshot && resyncFromRemoteRepo))
        {
            if (!ArtifactUtils.DownloadFromRemoteRepository(artifact, logger))
            {
                if (!artifactExists)
                {
                    RaiseError("Unable to get the artifact {0} from any of your repositories.", artifact.ArtifactId);
                    return;
                }
                else
                {
                    // TODO: warn about that
                }
            }
        }
        
        CopyToReferenceFolder(artifact, referenceFolder);
        

        I'll comment a bit more a bit later.

        Show
        Stoyan Damov added a comment - - edited I'll try to make it clearer: First, here's the new code: if (!ArtifactUtils.Exists(artifact) || (isSnapshot && resyncFromRemoteRepo)) { if (!ArtifactUtils.DownloadFromRemoteRepository(artifact, logger)) { RaiseError( "Unable to get the artifact {0} from any of your repositories." , artifact.ArtifactId); return ; } } CopyToReferenceFolder(artifact, referenceFolder); TODO : gotta go, will comment a bit later tonight but I see a flaw already, the code should read: bool artifactExists = ArtifactUtils.Exists(artifact); if (!artifactExists || (isSnapshot && resyncFromRemoteRepo)) { if (!ArtifactUtils.DownloadFromRemoteRepository(artifact, logger)) { if (!artifactExists) { RaiseError( "Unable to get the artifact {0} from any of your repositories." , artifact.ArtifactId); return ; } else { // TODO: warn about that } } } CopyToReferenceFolder(artifact, referenceFolder); I'll comment a bit more a bit later.
        Hide
        Lars Corneliussen added a comment -

        I still get an exception, if the artifact is not deployed to any of the remote repositories. That's not correct, is it?

        Both Resync and Resync from Local Repo should work in that case, right?

        Just that Resync from Local Repo should give local snapshots precedence over remote ones, even if the remote ones are newer. Right, too?

        Show
        Lars Corneliussen added a comment - I still get an exception, if the artifact is not deployed to any of the remote repositories. That's not correct, is it? Both Resync and Resync from Local Repo should work in that case, right? Just that Resync from Local Repo should give local snapshots precedence over remote ones, even if the remote ones are newer. Right, too?
        Hide
        Lars Corneliussen added a comment -

        I have committed the fix without changes. I'll test if it works on my machine next week.

        Show
        Lars Corneliussen added a comment - I have committed the fix without changes. I'll test if it works on my machine next week.
        Hide
        Stoyan Damov added a comment -

        Attached sure-fire reports files.

        Show
        Stoyan Damov added a comment - Attached sure-fire reports files.
        Hide
        Stoyan Damov added a comment -

        I just did, here's the output:

        -------------------------------------------------------
         T E S T S
        -------------------------------------------------------
        Running npanday.its.IntegrationTestSuite
        Using NPanday version 1.4.1-incubating-SNAPSHOT
        Available framework versions: [v2.0.50727, v4.0.30319]
        Selected framework version: v4.0.30319
        
        Bootstrap.testBootstrap                                               OK
        NPANDAY_377_WithCustomNPandaySettingsFile.test                        OK
        NPANDAY_377_WithCustomNPandaySettingsDirectory.test                   OK
        NPANDAY_329_VS2010WcfProjectSupport.testWCF2010Project                OK
        NPANDAY_328_VS2010WpfProjectSupport.testWPF2010Project                OK
        NPANDAY_330_VS2010MvcProjectSupport.testMVC2010Project                OK
        NPANDAY_288_Net40Support.testNet40Project                             OK
        NPANDAY_302_SnapshotUpdates.testUniqueSnapshotUpdates                 OK
        NPANDAY_292_CompilerParamForOptioninfer.testWithOptionInfer           OK
        NPANDAY_140_ConflictingExtensions.testConflictingExtensions
        NPANDAY_268_TransitiveDependencies.testLadderBuild                    OK
        NPANDAY_268_TransitiveDependencies.testTransitiveDependenciesNotOnCompileOK
        NPANDAY_262_ResolvingMixedVersions.testMixedVersionResolution         OK
        NPANDAY_196_MvcSupport.testMVCProject                                 OK
        NPANDAY_245_WpfGeneratedResourcesHandling.testWpfProject              OK
        NPANDAY_198_MissingGroupOrVersion.testMissingGroupIdAndVersionShouldBeInheritedOK
        NPANDAY_208_MsBuildCopyReferences.testMsBuildCopyReferences           OK
        NPANDAY_202_MsBuildErrorHandling.testMsBuildErrorsHandled             OK
        NPANDAY_121_ResGenWithErrorInFileName.testResGenWithErrorInFileName   OK
        NPandayIT0014WithResourceFile.testWithResourceFile                    OK
        NPandayIT0013WebAppInstall.testWebAppInstall                          OK
        NPandayIT0012VBWebApp.testWebAppInstall                               OK
        NPandayIT0011SnapshotResolution.testUniqueSnapshotResolution          OK
        NPandayIT0011SnapshotResolution.testNonUniqueSnapshotResolution       OK
        NPandayIT0041Net35.testNet35Project                                   OK
        NPandayIT0040IntraProjectDependency.testIntraProjectDependency        OK
        NPandayIT0039ConsoleApplication.testConsoleApplication                OK
        NPandayIT0038CompilerWithArgs.testCompilerWithArgs                    OK
        NPandayIT0037ClassLibWithWebRefInstall.testClassLibWithWebRefInstall  OK
        NPandayIT0036InstalledArtifactsVerification.testIT0036InstalledArtifactsOK
        NPandayIT0035VBRootNamespace.testVBRootNamespace                      OK
        NPandayIT0032CompileExclusions.testCompileExclusions                  OK
        NPandayIT0029RemoteRepo.testDeployNonSnapshotRemoteRepo               OK
        NPandayIT0028RemoteSnapshotRepo.testSnapDeploymentRemoteRepoNotUnique OK
        NPandayIT0022StrongNameKeyAddedToAssembly.testStrongNameKeyAddedToAssemblyOK
        NPandayIT0020EmbeddedResources.testEmbeddedResources                  OK
        NPandayIT0010VBCompilation.testVBCompilation                          OK
        NPandayIT0007XSDVerification.testGenerateXsdFromXml                   OK
        NPandayIT0006StockingHandlers.testXsdPlugin                           OK
        NPandayIT0004NUnitTestVerification.testNUnitTestsRun                  OK
        NPandayIT0001CompilerVerification.testCompiler                        OK@SLTests run: 41, Failures: 0, Errors: 1, Skipped: 0, Time elapsed:
        496.292 sec <<< FAILURE!
        
        Results :
        
        Tests in error:
          testConflictingExtensions(npanday.its.NPANDAY_140_ConflictingExtensionsTest)
        
        Tests run: 41, Failures: 0, Errors: 1, Skipped: 0
        
        [INFO] ------------------------------------------------------------------------
        [ERROR] BUILD FAILURE
        [INFO] ------------------------------------------------------------------------
        [INFO] There are test failures.
        
        Please refer to C:\Work\svn\npanday-its\target\surefire-reports for the individual test results.
        [INFO] ------------------------------------------------------------------------
        [INFO] For more information, run Maven with the -e switch
        [INFO] ------------------------------------------------------------------------
        [INFO] Total time: 8 minutes 18 seconds
        [INFO] Finished at: Thu Nov 03 15:35:17 EET 2011
        [INFO] Final Memory: 9M/22M
        [INFO] ------------------------------------------------------------------------
        C:\Work\svn\npanday-its>
        

        I'm not sure whether this is something which I broke or something known. Let me know if I should invest time to check or this is something known.
        I'll attach the surefire-reports files in a minute.

        Thanks,
        Stoyan

        Show
        Stoyan Damov added a comment - I just did, here's the output: ------------------------------------------------------- T E S T S ------------------------------------------------------- Running npanday.its.IntegrationTestSuite Using NPanday version 1.4.1-incubating-SNAPSHOT Available framework versions: [v2.0.50727, v4.0.30319] Selected framework version: v4.0.30319 Bootstrap.testBootstrap OK NPANDAY_377_WithCustomNPandaySettingsFile.test OK NPANDAY_377_WithCustomNPandaySettingsDirectory.test OK NPANDAY_329_VS2010WcfProjectSupport.testWCF2010Project OK NPANDAY_328_VS2010WpfProjectSupport.testWPF2010Project OK NPANDAY_330_VS2010MvcProjectSupport.testMVC2010Project OK NPANDAY_288_Net40Support.testNet40Project OK NPANDAY_302_SnapshotUpdates.testUniqueSnapshotUpdates OK NPANDAY_292_CompilerParamForOptioninfer.testWithOptionInfer OK NPANDAY_140_ConflictingExtensions.testConflictingExtensions NPANDAY_268_TransitiveDependencies.testLadderBuild OK NPANDAY_268_TransitiveDependencies.testTransitiveDependenciesNotOnCompileOK NPANDAY_262_ResolvingMixedVersions.testMixedVersionResolution OK NPANDAY_196_MvcSupport.testMVCProject OK NPANDAY_245_WpfGeneratedResourcesHandling.testWpfProject OK NPANDAY_198_MissingGroupOrVersion.testMissingGroupIdAndVersionShouldBeInheritedOK NPANDAY_208_MsBuildCopyReferences.testMsBuildCopyReferences OK NPANDAY_202_MsBuildErrorHandling.testMsBuildErrorsHandled OK NPANDAY_121_ResGenWithErrorInFileName.testResGenWithErrorInFileName OK NPandayIT0014WithResourceFile.testWithResourceFile OK NPandayIT0013WebAppInstall.testWebAppInstall OK NPandayIT0012VBWebApp.testWebAppInstall OK NPandayIT0011SnapshotResolution.testUniqueSnapshotResolution OK NPandayIT0011SnapshotResolution.testNonUniqueSnapshotResolution OK NPandayIT0041Net35.testNet35Project OK NPandayIT0040IntraProjectDependency.testIntraProjectDependency OK NPandayIT0039ConsoleApplication.testConsoleApplication OK NPandayIT0038CompilerWithArgs.testCompilerWithArgs OK NPandayIT0037ClassLibWithWebRefInstall.testClassLibWithWebRefInstall OK NPandayIT0036InstalledArtifactsVerification.testIT0036InstalledArtifactsOK NPandayIT0035VBRootNamespace.testVBRootNamespace OK NPandayIT0032CompileExclusions.testCompileExclusions OK NPandayIT0029RemoteRepo.testDeployNonSnapshotRemoteRepo OK NPandayIT0028RemoteSnapshotRepo.testSnapDeploymentRemoteRepoNotUnique OK NPandayIT0022StrongNameKeyAddedToAssembly.testStrongNameKeyAddedToAssemblyOK NPandayIT0020EmbeddedResources.testEmbeddedResources OK NPandayIT0010VBCompilation.testVBCompilation OK NPandayIT0007XSDVerification.testGenerateXsdFromXml OK NPandayIT0006StockingHandlers.testXsdPlugin OK NPandayIT0004NUnitTestVerification.testNUnitTestsRun OK NPandayIT0001CompilerVerification.testCompiler OK@SLTests run: 41, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 496.292 sec <<< FAILURE! Results : Tests in error: testConflictingExtensions(npanday.its.NPANDAY_140_ConflictingExtensionsTest) Tests run: 41, Failures: 0, Errors: 1, Skipped: 0 [INFO] ------------------------------------------------------------------------ [ERROR] BUILD FAILURE [INFO] ------------------------------------------------------------------------ [INFO] There are test failures. Please refer to C:\Work\svn\npanday-its\target\surefire-reports for the individual test results. [INFO] ------------------------------------------------------------------------ [INFO] For more information, run Maven with the -e switch [INFO] ------------------------------------------------------------------------ [INFO] Total time: 8 minutes 18 seconds [INFO] Finished at: Thu Nov 03 15:35:17 EET 2011 [INFO] Final Memory: 9M/22M [INFO] ------------------------------------------------------------------------ C:\Work\svn\npanday-its> I'm not sure whether this is something which I broke or something known. Let me know if I should invest time to check or this is something known. I'll attach the surefire-reports files in a minute. Thanks, Stoyan
        Hide
        Lars Corneliussen added a comment -

        Sure, comments would be fine.
        Also, have you run the integration tests locally?

        I'll test your patch locally tomorrow, and commit it, if it's fine.

        Show
        Lars Corneliussen added a comment - Sure, comments would be fine. Also, have you run the integration tests locally? I'll test your patch locally tomorrow, and commit it, if it's fine.
        Hide
        Stoyan Damov added a comment - - edited

        NPANDAY-322 fix

        Added ArtifactUtils helper class with the following methods:

        bool IsSnapshot(Artifact.Artifact artifact)
        

        Checks whether the artifact is a snapshot one, implementation checks whether artifact's version ends on -SNAPSHOT.

        bool Exists(Artifact.Artifact artifact)
        

        Checks whether the artifact exists in the local repository (impl. in terms of artifact.FileInfo.Exists)

        bool DownloadFromRemoteRepository(Artifact.Artifact artifact, NPanday.Logging.Logger logger)
        

        Downloads the artifact from the remote repository. Same as NPanday.ProjectImporter.Digest.Model.Reference.DownloadArtifact(artifact, logger).

        string GetArtifactReferenceFolder(Artifact.Artifact artifact, string referenceFolder)
        

        Returns the reference folder for the artifact, modified to match the .dll searched in NPanday.ProjectImporter.Digest.Model.Reference.cs. The format is referenceFolder\groupId\artifactId-version, e.g. SomeFolder\.references\com.company.something\artifact-1.0-SNAPSHOT

        string GetArtifactReferenceFilePath(Artifact.Artifact artifact, string referenceFolder)
        

        Returns path to the artifact's file in the local reference folder in the format GetArtifactReferenceFolder()\artifactId.extension, e.g. SomeFolder\.references\com.company.something\artifact-1.0-SNAPSHOT\artifact.dll.

        DateTime GetArtifactTimestamp(Artifact.Artifact artifact)
        

        Returs the artifact's timestamp in the local repository. Attempts to get the timestamp off maven-metadata-repoId.xml or maven-metadata-local.xml, falls back to file's LastWriteStampUtc.

        bool IsEarlierArtifactTimestamp(DateTime value, DateTime comparand)
        

        Compares two timestamps disregarding the milliseconds (timestamps in maven-metadata-*.xml do not have milliseconds).

        {note}I should have added the above documentation in the class. Let me know if I should do it and then re-attach the diff.{note}

        The fix explained:

        CopyArtifactImpl which is now called by the original CopyArtifact looks like this:

        private void CopyArtifactImpl(
            Artifact.Artifact artifact, 
            NPanday.Logging.Logger logger, 
            ArtifactResyncSource artifactResyncSource)
        {
            EnsureInitialized();
        
            bool isSnapshot = ArtifactUtils.IsSnapshot(artifact);
            bool resyncFromRemoteRepo = artifactResyncSource == ArtifactResyncSource.RemoteRepository;
        
            if (!ArtifactUtils.Exists(artifact) || (isSnapshot && resyncFromRemoteRepo))
            {
                if (!ArtifactUtils.DownloadFromRemoteRepository(artifact, logger))
                {
                    RaiseError("Unable to get the artifact {0} from any of your repositories.", artifact.ArtifactId);
                    return;
                }
            }
        
            CopyToReferenceFolder(artifact, referenceFolder);
        }
        

        The code above works towards implementing NPANDAY-476 (resync from local repo only) and then calls into CopyToReferenceFolder which now looks quite a bit different after the fix:

        static string CopyToReferenceFolder(Artifact.Artifact artifact, string referenceFolder)
        {
            string artifactReferenceFilePath = ArtifactUtils.GetArtifactReferenceFilePath(artifact, referenceFolder);
        
            bool overwriteReferenceFile;
            DateTime localRepoArtifactTimestamp = ArtifactUtils.GetArtifactTimestamp(artifact);
            if (File.Exists(artifactReferenceFilePath))
            {
                DateTime referenceFileTimestamp = new FileInfo(artifactReferenceFilePath).LastWriteTimeUtc;
                overwriteReferenceFile = ArtifactUtils.IsEarlierArtifactTimestamp(
                    referenceFileTimestamp, 
                    localRepoArtifactTimestamp);
            }
            else
            {
                overwriteReferenceFile = true;
            }
        
            if (overwriteReferenceFile)
            {
                File.Copy(artifact.FileInfo.FullName, artifactReferenceFilePath, true);
                // set the timestamp of the local repo's artifact
                new FileInfo(artifactReferenceFilePath).LastWriteTimeUtc = localRepoArtifactTimestamp;
            }
        
            return artifactReferenceFilePath;
        }
        

        The ArtifactUtils calls are explained at the top of this comment.

        Implemented NPANDAY-476

        I know it's wrong to put that in this ticket, but this is a placeholder until I get myself to update the other ticket.

        In Connect.cs refactored a bit so that methods dealing with resync solution/project references have appropriate names.
        Extracted the resync logic into implementation methods and refactored button click event handlers to call these.
        In the implementation methods added a boolean flag whether to resync from the remote or local repositories and then just delegated to the appropriate ReferenceManager.ResyncXxx call.

        In ReferenceManager.cs's IReferenceManager added ResyncArtifactsFromLocalRepository method.

        Refactored ResyncArtifacts and ResyncArtifactsFromLocalRepository to call an implementation method, passing whether the resync should be done from the remote or local repository.

        Show
        Stoyan Damov added a comment - - edited NPANDAY-322 fix Added ArtifactUtils helper class with the following methods: bool IsSnapshot(Artifact.Artifact artifact) Checks whether the artifact is a snapshot one, implementation checks whether artifact's version ends on -SNAPSHOT . bool Exists(Artifact.Artifact artifact) Checks whether the artifact exists in the local repository (impl. in terms of artifact.FileInfo.Exists ) bool DownloadFromRemoteRepository(Artifact.Artifact artifact, NPanday.Logging.Logger logger) Downloads the artifact from the remote repository. Same as NPanday.ProjectImporter.Digest.Model.Reference.DownloadArtifact(artifact, logger) . string GetArtifactReferenceFolder(Artifact.Artifact artifact, string referenceFolder) Returns the reference folder for the artifact, modified to match the .dll searched in NPanday.ProjectImporter.Digest.Model.Reference.cs . The format is referenceFolder\groupId\artifactId-version , e.g. SomeFolder\.references\com.company.something\artifact-1.0-SNAPSHOT string GetArtifactReferenceFilePath(Artifact.Artifact artifact, string referenceFolder) Returns path to the artifact's file in the local reference folder in the format GetArtifactReferenceFolder()\artifactId.extension , e.g. SomeFolder\.references\com.company.something\artifact-1.0-SNAPSHOT\artifact.dll . DateTime GetArtifactTimestamp(Artifact.Artifact artifact) Returs the artifact's timestamp in the local repository. Attempts to get the timestamp off maven-metadata-repoId.xml or maven-metadata-local.xml , falls back to file's LastWriteStampUtc . bool IsEarlierArtifactTimestamp(DateTime value, DateTime comparand) Compares two timestamps disregarding the milliseconds (timestamps in maven-metadata-*.xml do not have milliseconds). {note}I should have added the above documentation in the class. Let me know if I should do it and then re-attach the diff.{note} The fix explained: CopyArtifactImpl which is now called by the original CopyArtifact looks like this: private void CopyArtifactImpl( Artifact.Artifact artifact, NPanday.Logging.Logger logger, ArtifactResyncSource artifactResyncSource) { EnsureInitialized(); bool isSnapshot = ArtifactUtils.IsSnapshot(artifact); bool resyncFromRemoteRepo = artifactResyncSource == ArtifactResyncSource.RemoteRepository; if (!ArtifactUtils.Exists(artifact) || (isSnapshot && resyncFromRemoteRepo)) { if (!ArtifactUtils.DownloadFromRemoteRepository(artifact, logger)) { RaiseError( "Unable to get the artifact {0} from any of your repositories." , artifact.ArtifactId); return ; } } CopyToReferenceFolder(artifact, referenceFolder); } The code above works towards implementing NPANDAY-476 (resync from local repo only) and then calls into CopyToReferenceFolder which now looks quite a bit different after the fix: static string CopyToReferenceFolder(Artifact.Artifact artifact, string referenceFolder) { string artifactReferenceFilePath = ArtifactUtils.GetArtifactReferenceFilePath(artifact, referenceFolder); bool overwriteReferenceFile; DateTime localRepoArtifactTimestamp = ArtifactUtils.GetArtifactTimestamp(artifact); if (File.Exists(artifactReferenceFilePath)) { DateTime referenceFileTimestamp = new FileInfo(artifactReferenceFilePath).LastWriteTimeUtc; overwriteReferenceFile = ArtifactUtils.IsEarlierArtifactTimestamp( referenceFileTimestamp, localRepoArtifactTimestamp); } else { overwriteReferenceFile = true ; } if (overwriteReferenceFile) { File.Copy(artifact.FileInfo.FullName, artifactReferenceFilePath, true ); // set the timestamp of the local repo's artifact new FileInfo(artifactReferenceFilePath).LastWriteTimeUtc = localRepoArtifactTimestamp; } return artifactReferenceFilePath; } The ArtifactUtils calls are explained at the top of this comment. Implemented NPANDAY-476 I know it's wrong to put that in this ticket, but this is a placeholder until I get myself to update the other ticket. In Connect.cs refactored a bit so that methods dealing with resync solution/project references have appropriate names. Extracted the resync logic into implementation methods and refactored button click event handlers to call these. In the implementation methods added a boolean flag whether to resync from the remote or local repositories and then just delegated to the appropriate ReferenceManager.ResyncXxx call. In ReferenceManager.cs 's IReferenceManager added ResyncArtifactsFromLocalRepository method. Refactored ResyncArtifacts and ResyncArtifactsFromLocalRepository to call an implementation method, passing whether the resync should be done from the remote or local repository.
        Hide
        Stoyan Damov added a comment -

        ReferenceManager.cs and ArtifactUtils.cs fix NPANDAY-322.

        ReferenceManager.cs also has the code necessary to implement NPANDAY-476.

        I've also attached Connect.cs which does implement NPANDAY-476.

        This is my first contribution so I'm not really sure whether I can propose a change set for two issues at a time. Happy to split the changeset in two if necessary.

        I'm going to put a separate comment explaining all changes.

        Show
        Stoyan Damov added a comment - ReferenceManager.cs and ArtifactUtils.cs fix NPANDAY-322 . ReferenceManager.cs also has the code necessary to implement NPANDAY-476 . I've also attached Connect.cs which does implement NPANDAY-476 . This is my first contribution so I'm not really sure whether I can propose a change set for two issues at a time. Happy to split the changeset in two if necessary. I'm going to put a separate comment explaining all changes.
        Hide
        Stoyan Damov added a comment -

        The fix which I'm about to attach to this ticket (btw what should be it? a .patch or a .diff file?) reverts to File.Copy.

        Show
        Stoyan Damov added a comment - The fix which I'm about to attach to this ticket (btw what should be it? a .patch or a .diff file?) reverts to File.Copy.
        Hide
        Lars Corneliussen added a comment -

        I can't see, why File.Copy has been changed..
        Who made the change? Ask the committer why he made this change..

        Also I really, sorry, hate the silent-catches all over NPanday. This is an error - not showing it, doesn't make it any better.

        Show
        Lars Corneliussen added a comment - I can't see, why File.Copy has been changed.. Who made the change? Ask the committer why he made this change.. Also I really, sorry, hate the silent-catches all over NPanday. This is an error - not showing it, doesn't make it any better.
        Hide
        Stoyan Damov added a comment -

        I checked out the trunk and it's a bit worse there because the actual file copy in ReferenceManager.CopyToReferenceFolder changed from:

        File.Copy(artifact.FileInfo.FullName, artifactFileName, true);
        

        to:

        try
        {
            byte[] contents = File.ReadAllBytes(artifact.FileInfo.FullName);
            File.WriteAllBytes(artifactFileName, contents);
        }
        catch (Exception ex)
        {
            Console.WriteLine(ex.ToString());
        }
        

        Ignoring the Console.WriteLine call (and postponing the why?), the code above creates a new file (with a new last write timestamp) in the .references folder which means that if the artifact got updated between the resync and the actual copy, next time the check for a sooner timestamp will fail.

        The best thing to do is to not deal with file timestamps at all, and instead read maven-metadata*.xml files in the local repo and get the timestamp from there. This also means that copying the artifact to the .references folder should either copy the .xml files as well, or create a timestamp file or at least change the file timestamp to the one from the xml(s).

        What do you think about that?

        Other than that, I already started working on the patch (deleted the quick & dirty one I had in order to make something which will hopefully be accepted for commit).

        Show
        Stoyan Damov added a comment - I checked out the trunk and it's a bit worse there because the actual file copy in ReferenceManager.CopyToReferenceFolder changed from: File.Copy(artifact.FileInfo.FullName, artifactFileName, true ); to: try { byte [] contents = File.ReadAllBytes(artifact.FileInfo.FullName); File.WriteAllBytes(artifactFileName, contents); } catch (Exception ex) { Console.WriteLine(ex.ToString()); } Ignoring the Console.WriteLine call (and postponing the why? ), the code above creates a new file (with a new last write timestamp) in the .references folder which means that if the artifact got updated between the resync and the actual copy, next time the check for a sooner timestamp will fail. The best thing to do is to not deal with file timestamps at all, and instead read maven-metadata*.xml files in the local repo and get the timestamp from there. This also means that copying the artifact to the .references folder should either copy the .xml files as well, or create a timestamp file or at least change the file timestamp to the one from the xml(s). What do you think about that? Other than that, I already started working on the patch (deleted the quick & dirty one I had in order to make something which will hopefully be accepted for commit).
        Hide
        Lars Corneliussen added a comment -

        I agree; I even stumbled upon the same problem just a couple of days ago.

        Would you consider submitting a patch we could then apply?

        Show
        Lars Corneliussen added a comment - I agree; I even stumbled upon the same problem just a couple of days ago. Would you consider submitting a patch we could then apply?
        Hide
        Stoyan Damov added a comment -

        I can't re-open the ticket but the problem is still there in NPanDay.VisualStudio.Plugin (mainly in ReferenceManager).

        In ReferenceManager.CopyArtifact there's this snippet

        if (!artifact.FileInfo.Exists || artifact.Version.EndsWith("SNAPSHOT"))
        {
            if (!NPanday.ProjectImporter.Digest.Model.Reference.DownloadArtifact(artifact,logger))
            {
                ReferenceErrorEventArgs e = new ReferenceErrorEventArgs();
                e.Message = string.Format("Unable to get the artifact {0} from any of your repositories.", artifact.ArtifactId);
                onError(e);
                return;
            }
        }
        

        The check for SNAPSHOT above forces "Resync references" to try and download the artifact off the remote repository.
        If the artifact is not yet installed (which is common if I'm developing a new module but don't want to commit/push it yet), what happens is that:

        1. The resync fails (which is bad) and
        2. If the artifact is installed in the local repo, it gets deleted (which is
          worse)

        What should happen (for SNAPSHOTs) instead is this:

        1. Try to get timestamp of artifact in remote repo. If that fails, it shouldn't be a problem, the artifact might not be deployed (yet).
        2. Try to get timestamp of artifact in local repo. If that fails, then this is an error and should be reported.
        3. Get the timestamp of the local file (in .references/...)
        4. If there's a remote timestamp and it's greater than the local one, update the local repo's artifact.
        5. If the local repo's timestamp (possibly updated at point 4) is greater than the timestamp of the local file, update the file in .references/...

        For steps 1 and 2 use the timestamp in maven-metadata(-local?).xml (in ReferenceManager.copyToReferenceFolder there's already a TODO which suggests
        to use metadata/versioning/lastUpdated for the timestamp). Currently, the local repo's timestamp is considered to be the file's last write time (not even .LastWriteTimeUtc, which is another bug).

        To make it clear, since we're all developers, here's the algo in pseudo code (this might not compile, I'm writing it in Jira):

        DateTime remoteRepoTimestamp, localRepoTimestamp, localFileTimestamp;
        bool hasRemoteTimestamp =TryGetArtifactRemoteRepositoryTimestamp(artifact, logger, out remoteRepoTimestamp);
        bool hasLocalRepoTimestamp = TryGetArtifactLocalRepositoryTimestamp(artifact, logger, out localRepoTimestamp);
        localFileTimestamp = GetLocalFileTimestamp(artifact.FileInfo, logger);
        
        if (hasRemoteTimestamp && hasLocalRepoTimestamp && remoteRepoTimestamp > localRepoTimestamp)
        {
            UpdateLocalRepoArtifact(); // == NPanday.ProjectImporter.Digest.Model.Reference.DownloadArtifact(artifact, logger)
            // ^^ error handling above omitted for brevity
            localRepoTimestamp = remoteRepoTimestamp;
        }
        if (hasLocalRepoTimestamp && localRepoTimestamp > localFileTimestamp)
        {
            copyToReferenceFolder(artifact, referenceFolder);
        }
        

        Something like this.

        Show
        Stoyan Damov added a comment - I can't re-open the ticket but the problem is still there in NPanDay.VisualStudio.Plugin (mainly in ReferenceManager). In ReferenceManager.CopyArtifact there's this snippet if (!artifact.FileInfo.Exists || artifact.Version.EndsWith( "SNAPSHOT" )) { if (!NPanday.ProjectImporter.Digest.Model.Reference.DownloadArtifact(artifact,logger)) { ReferenceErrorEventArgs e = new ReferenceErrorEventArgs(); e.Message = string.Format( "Unable to get the artifact {0} from any of your repositories." , artifact.ArtifactId); onError(e); return ; } } The check for SNAPSHOT above forces "Resync references" to try and download the artifact off the remote repository. If the artifact is not yet installed (which is common if I'm developing a new module but don't want to commit/push it yet), what happens is that: The resync fails (which is bad) and If the artifact is installed in the local repo, it gets deleted (which is worse) What should happen (for SNAPSHOTs) instead is this: Try to get timestamp of artifact in remote repo. If that fails, it shouldn't be a problem, the artifact might not be deployed (yet). Try to get timestamp of artifact in local repo. If that fails, then this is an error and should be reported. Get the timestamp of the local file (in .references/...) If there's a remote timestamp and it's greater than the local one, update the local repo's artifact. If the local repo's timestamp (possibly updated at point 4) is greater than the timestamp of the local file, update the file in .references/... For steps 1 and 2 use the timestamp in maven-metadata(-local?).xml (in ReferenceManager.copyToReferenceFolder there's already a TODO which suggests to use metadata/versioning/lastUpdated for the timestamp). Currently, the local repo's timestamp is considered to be the file's last write time (not even .LastWriteTimeUtc , which is another bug). To make it clear, since we're all developers, here's the algo in pseudo code (this might not compile, I'm writing it in Jira): DateTime remoteRepoTimestamp, localRepoTimestamp, localFileTimestamp; bool hasRemoteTimestamp =TryGetArtifactRemoteRepositoryTimestamp(artifact, logger, out remoteRepoTimestamp); bool hasLocalRepoTimestamp = TryGetArtifactLocalRepositoryTimestamp(artifact, logger, out localRepoTimestamp); localFileTimestamp = GetLocalFileTimestamp(artifact.FileInfo, logger); if (hasRemoteTimestamp && hasLocalRepoTimestamp && remoteRepoTimestamp > localRepoTimestamp) { UpdateLocalRepoArtifact(); // == NPanday.ProjectImporter.Digest.Model.Reference.DownloadArtifact(artifact, logger) // ^^ error handling above omitted for brevity localRepoTimestamp = remoteRepoTimestamp; } if (hasLocalRepoTimestamp && localRepoTimestamp > localFileTimestamp) { copyToReferenceFolder(artifact, referenceFolder); } Something like this.

          People

          • Assignee:
            Lars Corneliussen
            Reporter:
            Dmitry L
          • Votes:
            1 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development