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

Missing base directory in ZIP from .../artifact/dir/subdir/*zip*/subdir.zip

    XMLWordPrintable

    Details

    • Similar Issues:

      Description

      1.509.x would create a subdir.zip with a root entry subdir/. Development builds omit this top directory.

        Attachments

          Issue Links

            Activity

            jglick Jesse Glick created issue -
            jglick Jesse Glick made changes -
            Field Original Value New Value
            Link This issue is blocking JENKINS-17236 [ JENKINS-17236 ]
            jglick Jesse Glick made changes -
            Link This issue is related to JENKINS-19752 [ JENKINS-19752 ]
            jglick Jesse Glick made changes -
            Status Open [ 1 ] In Progress [ 3 ]
            Hide
            scm_issue_link SCM/JIRA link daemon added a comment -

            Code changed in jenkins
            User: Jesse Glick
            Path:
            changelog.html
            core/src/main/java/hudson/FilePath.java
            core/src/main/java/hudson/model/DirectoryBrowserSupport.java
            core/src/main/java/hudson/util/DirScanner.java
            test/src/test/java/hudson/model/DirectoryBrowserSupportTest.java
            http://jenkins-ci.org/commit/jenkins/793b6826e567e20a256a6d4166031e75a185ef7a
            Log:
            [FIXED JENKINS-19947] Restore historical undocumented behavior of a top-level entry in a ZIP.
            Noting that various DirScanner implementations handle this inconsistently.

            Show
            scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Jesse Glick Path: changelog.html core/src/main/java/hudson/FilePath.java core/src/main/java/hudson/model/DirectoryBrowserSupport.java core/src/main/java/hudson/util/DirScanner.java test/src/test/java/hudson/model/DirectoryBrowserSupportTest.java http://jenkins-ci.org/commit/jenkins/793b6826e567e20a256a6d4166031e75a185ef7a Log: [FIXED JENKINS-19947] Restore historical undocumented behavior of a top-level entry in a ZIP. Noting that various DirScanner implementations handle this inconsistently.
            scm_issue_link SCM/JIRA link daemon made changes -
            Status In Progress [ 3 ] Resolved [ 5 ]
            Resolution Fixed [ 1 ]
            Hide
            dogfood dogfood added a comment -

            Integrated in jenkins_main_trunk #2940
            [FIXED JENKINS-19947] Restore historical undocumented behavior of a top-level entry in a ZIP. (Revision 793b6826e567e20a256a6d4166031e75a185ef7a)

            Result = UNSTABLE
            Jesse Glick : 793b6826e567e20a256a6d4166031e75a185ef7a
            Files :

            • changelog.html
            • core/src/main/java/hudson/util/DirScanner.java
            • test/src/test/java/hudson/model/DirectoryBrowserSupportTest.java
            • core/src/main/java/hudson/FilePath.java
            • core/src/main/java/hudson/model/DirectoryBrowserSupport.java
            Show
            dogfood dogfood added a comment - Integrated in jenkins_main_trunk #2940 [FIXED JENKINS-19947] Restore historical undocumented behavior of a top-level entry in a ZIP. (Revision 793b6826e567e20a256a6d4166031e75a185ef7a) Result = UNSTABLE Jesse Glick : 793b6826e567e20a256a6d4166031e75a185ef7a Files : changelog.html core/src/main/java/hudson/util/DirScanner.java test/src/test/java/hudson/model/DirectoryBrowserSupportTest.java core/src/main/java/hudson/FilePath.java core/src/main/java/hudson/model/DirectoryBrowserSupport.java
            Hide
            danielbeck Daniel Beck added a comment -

            Is this really something that should be changed? I've always wondered why the unnecessary directory was added in the first place.

            Show
            danielbeck Daniel Beck added a comment - Is this really something that should be changed? I've always wondered why the unnecessary directory was added in the first place.
            Hide
            jglick Jesse Glick added a comment -

            Is this really something that should be changed?

            It is an inconsistent special behavior that I am not fond of either, but as the historical behavior it must be retained as people may have scripts etc. relying on it.

            why the unnecessary directory was added in the first place

            Possibly by mistake (the code in question seems to have never previously been subject to automated test), but also possibly to ensure that someone running unzip or the equivalent will always get a single folder as a result—if you forget to unzip to a new destination directory, it can be quite annoying to have your current directory strewn with several dozen new files and subdirectories.

            Show
            jglick Jesse Glick added a comment - Is this really something that should be changed? It is an inconsistent special behavior that I am not fond of either, but as the historical behavior it must be retained as people may have scripts etc. relying on it. why the unnecessary directory was added in the first place Possibly by mistake (the code in question seems to have never previously been subject to automated test), but also possibly to ensure that someone running unzip or the equivalent will always get a single folder as a result—if you forget to unzip to a new destination directory, it can be quite annoying to have your current directory strewn with several dozen new files and subdirectories.
            Hide
            mwebber Matthew Webber added a comment -

            Possibly by mistake (the code in question seems to have never previously been subject to automated test), but also possibly to ensure that someone running unzip or the equivalent will always get a single folder as a result—if you forget to unzip to a new destination directory, it can be quite annoying to have your current directory strewn with several dozen new files and subdirectories

            Actually, unzip behaviour differs depending on whether you are Linux or Windows.
            On Linux, the behaviour is as you describe - the unzip writes into the current directory.
            On Windows, unzip creates a new directory with the same name as the .zip file (minus the .zip extension) and then writes into there (that's with the GUI - not sure what the command line does).

            In fact, when we zip up our product, we add an extra directory level in the Linux .zip, but not in the Windows .zip, for exactly that reason.

            Matthew

            Show
            mwebber Matthew Webber added a comment - Possibly by mistake (the code in question seems to have never previously been subject to automated test), but also possibly to ensure that someone running unzip or the equivalent will always get a single folder as a result—if you forget to unzip to a new destination directory, it can be quite annoying to have your current directory strewn with several dozen new files and subdirectories Actually, unzip behaviour differs depending on whether you are Linux or Windows. On Linux, the behaviour is as you describe - the unzip writes into the current directory. On Windows, unzip creates a new directory with the same name as the .zip file (minus the .zip extension) and then writes into there (that's with the GUI - not sure what the command line does). In fact, when we zip up our product, we add an extra directory level in the Linux .zip, but not in the Windows .zip, for exactly that reason. Matthew
            Hide
            danielbeck Daniel Beck added a comment - - edited

            For completeness: OS X (Archive Utility and The Unarchiver) creates a directory named like the zip file when there's more than one top level item in the archive. Otherwise, extracts into the current directory.

            Couldn't we retain the existing behavior for the URL, assuming scripts get their input from /*zip*/ directly, add /*fileszip*/ and only link to the latter on the UI? Or distinguish on the UI between 'files as zip' (without top-level dir) and 'current directory as zip' (old behavior)?

            Another option might be to make this behavior configurable as a UserProperty.

            Show
            danielbeck Daniel Beck added a comment - - edited For completeness: OS X (Archive Utility and The Unarchiver) creates a directory named like the zip file when there's more than one top level item in the archive. Otherwise, extracts into the current directory. Couldn't we retain the existing behavior for the URL, assuming scripts get their input from /*zip*/ directly, add /*fileszip*/ and only link to the latter on the UI? Or distinguish on the UI between 'files as zip' (without top-level dir) and 'current directory as zip' (old behavior)? Another option might be to make this behavior configurable as a UserProperty.
            Hide
            jglick Jesse Glick added a comment -

            I cannot quite follow the logic in DirectoryBrowserSupport, but it may be possible to append a glob ** to the URL somewhere (as the rest variable), in which case GlobScanner switches to the behavior of not including the parent dir. Anyway such possibilities would be better discussed in a separate RFE; this bug was just about an unintended change of behavior with the new implementation, now reverted.

            Show
            jglick Jesse Glick added a comment - I cannot quite follow the logic in DirectoryBrowserSupport , but it may be possible to append a glob ** to the URL somewhere (as the rest variable), in which case GlobScanner switches to the behavior of not including the parent dir. Anyway such possibilities would be better discussed in a separate RFE; this bug was just about an unintended change of behavior with the new implementation, now reverted.
            Hide
            danielbeck Daniel Beck added a comment - - edited

            Jesse: Thanks for the hint!

            For reference, /path/to/dir/**/*zip*/foo.zip downloads a file named foo.zip with the contents of /path/to/dir (i.e. no top-level directory named dir in the zip file). /path/to/dir/*/*zip*/foo.zip downloads all the files in the directory dir, without sub-directories.

            To conveniently download these, enter the pattern ** or * into the box in the path bar, see a preview of what you're going to get, and only then click the download link.

            Show
            danielbeck Daniel Beck added a comment - - edited Jesse: Thanks for the hint! For reference, /path/to/dir/**/*zip*/foo.zip downloads a file named foo.zip with the contents of /path/to/dir (i.e. no top-level directory named dir in the zip file). /path/to/dir/*/*zip*/foo.zip downloads all the files in the directory dir , without sub-directories. To conveniently download these, enter the pattern ** or * into the box in the path bar, see a preview of what you're going to get, and only then click the download link.
            Hide
            jglick Jesse Glick added a comment -

            Thought it was something like that. So then the issue boils down to providing a more discoverable UI for this feature, or picking a better default behavior, etc.

            Show
            jglick Jesse Glick added a comment - Thought it was something like that. So then the issue boils down to providing a more discoverable UI for this feature, or picking a better default behavior, etc.
            olivergondza Oliver Gondža made changes -
            Labels artifact regression artifact lts-candidate regression
            Hide
            eyeamvic Victor Baca added a comment -

            This issue may have caused JENKINS-19752 to become an issue again. I am unable to validate that this issue is resolved due to the now corrupt ZIP file.

            Show
            eyeamvic Victor Baca added a comment - This issue may have caused JENKINS-19752 to become an issue again. I am unable to validate that this issue is resolved due to the now corrupt ZIP file.
            Hide
            scm_issue_link SCM/JIRA link daemon added a comment -

            Code changed in jenkins
            User: Jesse Glick
            Path:
            core/src/main/java/hudson/FilePath.java
            core/src/main/java/hudson/model/DirectoryBrowserSupport.java
            core/src/main/java/hudson/util/DirScanner.java
            test/src/test/java/hudson/model/DirectoryBrowserSupportTest.java
            http://jenkins-ci.org/commit/jenkins/3695e614ed51c344bcc7deafaee44b11f7d1151d
            Log:
            [FIXED JENKINS-19947] Restore historical undocumented behavior of a top-level entry in a ZIP.
            Noting that various DirScanner implementations handle this inconsistently.

            (cherry picked from commit 793b6826e567e20a256a6d4166031e75a185ef7a)

            Conflicts:
            changelog.html

            Show
            scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Jesse Glick Path: core/src/main/java/hudson/FilePath.java core/src/main/java/hudson/model/DirectoryBrowserSupport.java core/src/main/java/hudson/util/DirScanner.java test/src/test/java/hudson/model/DirectoryBrowserSupportTest.java http://jenkins-ci.org/commit/jenkins/3695e614ed51c344bcc7deafaee44b11f7d1151d Log: [FIXED JENKINS-19947] Restore historical undocumented behavior of a top-level entry in a ZIP. Noting that various DirScanner implementations handle this inconsistently. (cherry picked from commit 793b6826e567e20a256a6d4166031e75a185ef7a) Conflicts: changelog.html
            olivergondza Oliver Gondža made changes -
            Labels artifact lts-candidate regression 1.532.1-fixed artifact regression
            tszadel Thomas Szadel made changes -
            Labels 1.532.1-fixed artifact regression 1.532.1-fixed artifact lts-candidate regression
            Hide
            danielbeck Daniel Beck added a comment -

            Next LTS will be based on 1.554, which contains the original fix, so no backporting required.

            Show
            danielbeck Daniel Beck added a comment - Next LTS will be based on 1.554, which contains the original fix, so no backporting required.
            danielbeck Daniel Beck made changes -
            Labels 1.532.1-fixed artifact lts-candidate regression 1.532.1-fixed artifact regression
            snakedoc Jason Sipula made changes -
            Link This issue is related to JENKINS-26700 [ JENKINS-26700 ]
            rtyler R. Tyler Croy made changes -
            Workflow JNJira [ 151468 ] JNJira + In-Review [ 193943 ]
            Hide
            jameshowe James Howe added a comment -

            I don't suppose there's any option if I want the directory to be omitted?

            Show
            jameshowe James Howe added a comment - I don't suppose there's any option if I want the directory to be omitted?
            Hide
            jglick Jesse Glick added a comment -

            James Howe use ** as an include pattern rather than omitting this parameter or leaving it blank.

            Show
            jglick Jesse Glick added a comment - James Howe use ** as an include pattern rather than omitting this parameter or leaving it blank.

              People

              • Assignee:
                jglick Jesse Glick
                Reporter:
                jglick Jesse Glick
              • Votes:
                1 Vote for this issue
                Watchers:
                7 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: