Uploaded image for project: 'Jenkins'
  1. Jenkins
  2. JENKINS-40912

Add verbose option to stash/unstash to get list of files

    Details

    • Sprint:
      Pipeline - October, Pipeline - April 2018
    • Similar Issues:

      Description

      It would be great if we could get more info on stash/unstash so I could see a list of files included. I was recently hit by an apparent unstashing hiccup where 1 file that should have been unstashed was not actually there and it made the subsequent errors rather confusing.

      Had there been a list of files in the log that were successfully unstashed then it would have been more clear what the problem was. Perhaps there could even be a validation step to go with this where if the number of files unstashed doesnt match number of stashed files then it logs an error.

        Attachments

          Issue Links

            Activity

            Hide
            abayer Andrew Bayer added a comment -

            Jesse Glick - so with the changes over on https://github.com/jenkinsci/workflow-api-plugin/pull/67, it feels like what would make the most sense is maybe for StashAwareArtifactManager to have a listStashFiles method - with StandardArtifactManager doing something like https://github.com/jenkinsci/workflow-api-plugin/pull/62/files#diff-4ad53e530bca0f047ecfd41828c4e0eeR182, but other StashAwareArtifactManager implementations doing something else. Does that make any sense?

            Show
            abayer Andrew Bayer added a comment - Jesse Glick - so with the changes over on https://github.com/jenkinsci/workflow-api-plugin/pull/67 , it feels like what would make the most sense is maybe for StashAwareArtifactManager to have a listStashFiles method - with StandardArtifactManager doing something like https://github.com/jenkinsci/workflow-api-plugin/pull/62/files#diff-4ad53e530bca0f047ecfd41828c4e0eeR182 , but other StashAwareArtifactManager implementations doing something else. Does that make any sense?
            Hide
            jglick Jesse Glick added a comment -

            Andrew Bayer I am still guessing that the basic user requirement would be satisfied by a verbose: true flag to the stash step. Once StashAwareArtifactManager lands, there could be multiple implementations, but at any rate the obvious way would be for FileVisitor to have a convenience method

            public FileVisitor verbose(TaskListener listener) {…}
            

            with the standard implementation being

            int count = workspace.archive(ArchiverFactory.TARGZ.verbose(listener), os, new DirScanner.Glob(/* as before… */));
            

            If you wish to avoid a new core dep (but see JEP-301!), the verbose method could temporarily be hosted as static in StashManager.

            My comment in PR 67 was just to the effect that if you agree with that design and want this RFE now, the easiest way to avoid clashes in introduced APIs would be to extract the new StashManager method overloads from that PR into a simple PR with no core dep change or StashAwareArtifactManager, including the added TaskListener parameters and the boolean verbose flag. Similarly, the StashStep and UnstashStep patches from workflow-basic-steps PR 60 would be extracted into a small PR that also adds the verbose step parameter. All easy for me to do (~30m) when I have this stuff in flight anyway, but I am not going to bother unless you are prepared to review and merge it.

            All that said, this whole RFE has a trivial workaround (I would hesitate to even call it that; more a diagnostic idiom): in your Jenkinsfile after an apparently misconfigured stash, when you unstash just

            sh 'ls -R'
            

            to see what you got and whether it matches your expectations. Optionally do the same in the original workspace prior to the stash and compare. Short of reimplementing the Ant patternset matching code from scratch to offer fine-grained explanations of why each individual candidate relative path did or did not match each component of each pattern, no feature we offer is really going to change the diagnostic experience much: you just experiment with whether a given patternset works or not, accompanied if necessary with a bit of stackoverflow.com searching, documentation review, and, well, thinking.

            Show
            jglick Jesse Glick added a comment - Andrew Bayer I am still guessing that the basic user requirement would be satisfied by a verbose: true flag to the stash step. Once StashAwareArtifactManager lands, there could be multiple implementations, but at any rate the obvious way would be for FileVisitor to have a convenience method public FileVisitor verbose(TaskListener listener) {…} with the standard implementation being int count = workspace.archive(ArchiverFactory.TARGZ.verbose(listener), os, new DirScanner.Glob( /* as before… */ )); If you wish to avoid a new core dep (but see JEP-301!), the verbose method could temporarily be hosted as static in StashManager . My comment in PR 67 was just to the effect that if you agree with that design and want this RFE now , the easiest way to avoid clashes in introduced APIs would be to extract the new StashManager method overloads from that PR into a simple PR with no core dep change or StashAwareArtifactManager , including the added TaskListener parameters and the boolean verbose flag. Similarly, the StashStep and UnstashStep patches from workflow-basic-steps PR 60 would be extracted into a small PR that also adds the verbose step parameter. All easy for me to do (~30m) when I have this stuff in flight anyway, but I am not going to bother unless you are prepared to review and merge it. All that said, this whole RFE has a trivial workaround (I would hesitate to even call it that; more a diagnostic idiom): in your Jenkinsfile after an apparently misconfigured stash , when you unstash just sh 'ls -R' to see what you got and whether it matches your expectations. Optionally do the same in the original workspace prior to the stash and compare. Short of reimplementing the Ant patternset matching code from scratch to offer fine-grained explanations of why each individual candidate relative path did or did not match each component of each pattern, no feature we offer is really going to change the diagnostic experience much: you just experiment with whether a given patternset works or not, accompanied if necessary with a bit of stackoverflow.com searching, documentation review, and, well, thinking.
            Hide
            jglick Jesse Glick added a comment -

            Actually verbose would need to be on ArchiverFactory I suppose.

            An alternative impl API would be for FilePath.archive to get an overload returning a List<String> paths, which could be used by callers to print diagnostics (though not in real time, and without the possibility of distinguishing files vs. directories vs. symlinks in output).

            Show
            jglick Jesse Glick added a comment - Actually verbose would need to be on ArchiverFactory I suppose. An alternative impl API would be for FilePath.archive to get an overload returning a List<String> paths, which could be used by callers to print diagnostics (though not in real time, and without the possibility of distinguishing files vs. directories vs. symlinks in output).
            Hide
            jg_lgc Justin Georgeson added a comment -

            My preference would be for a verbose flag on both stash and unstash. Even just having the unstash print out just the number of files (to match it up with the output stash currently provides) would be an improvement

            Show
            jg_lgc Justin Georgeson added a comment - My preference would be for a verbose flag on both stash and unstash. Even just having the unstash print out just the number of files (to match it up with the output stash currently provides) would be an improvement
            Hide
            jglick Jesse Glick added a comment -

            Even printing a count would require a core API change, I think. Not positive.

            Show
            jglick Jesse Glick added a comment - Even printing a count would require a core API change, I think. Not positive.

              People

              • Assignee:
                abayer Andrew Bayer
                Reporter:
                sparklepony Sparkle Pony
              • Votes:
                11 Vote for this issue
                Watchers:
                17 Start watching this issue

                Dates

                • Created:
                  Updated: