• Icon: Bug Bug
    • Resolution: Fixed
    • Icon: Major Major
    • core
    • None
    • Centos 5.6, 32-bit, JDK 7u3, Jenkins 1.455

      I've seen several related and/or resolved issue on this but I wasn't sure which one was relevant for re-opening.

      I have a very basic job (name DEV) that polls a git repository, deploys the files to a local webserver via a shell script, then archives the workspace. A second job then picks up the last successful archive w/ the Copy Artifact plugin and attempts to run the same deploy script and receives an error due to missing executable permissions. The intent is to put a skeleton together of a pipeline for DEV->QA->PROD, each deploying to a local folder (symlinked to a directory apache is aware of), the file permissions are deployment scripts are managed in the repository with the source for the site.

      I initially thought this was an issue with the Copy Artifact plugin, but after writing a basic shell script to manually copy the files and then looking directly at the archive folders in the job folders, it appears to be the Archive operation itself.

      It appears that all the file and folder permissions are being set/cleared when the archive occurred. The contents of the DEV workspace has original permissions after a run, but the archive contents (jenkins\jobs\DEV\builds##\archive) are missing their permissions. All files have been set to 644, all directories to 755 (>2000 files in multiple subdirectories, although to be fair I've only checked 5 or 6 subdirectories).

      This is similar to the related set of tickets under JENKINS-9397 but I wasn't sure which (if any) was best to reopen.

          [JENKINS-13128] Artifacts Permissions Stripped

          Oleg Nenashev added a comment -

          Unfinished pull request is here: https://github.com/jenkinsci/jenkins/pull/1576 . Everybody is welcome to take it and finalize it

          Oleg Nenashev added a comment - Unfinished pull request is here: https://github.com/jenkinsci/jenkins/pull/1576 . Everybody is welcome to take it and finalize it

          oleg_nenashev I see that it's stuck in a merge conflict. Even if that conflict would be solved, will that ever get merged in?

          IMO the proper solution is to simply NOT STRIP file properties, and just keep them as they are. It should assume that the developer who made the job knows what they are doing, and expects things to happen as he set them.

          Ovidiu-Florin Bogdan added a comment - oleg_nenashev I see that it's stuck in a merge conflict. Even if that conflict would be solved, will that ever get merged in? IMO the proper solution is to simply NOT STRIP file properties, and just keep them as they are. It should assume that the developer who made the job knows what they are doing, and expects things to happen as he set them.

          Daniel Beck added a comment -

          Even if that conflict would be solved, will that ever get merged in?

          I'm with Oliver (see comments there) on the proposed change, there seems to be no real use case to not preserve attributes, so the options seem unnecessary. Should be straightforward to rip that part out of the PR, resolve the conflicts, and resubmit for review.

          Daniel Beck added a comment - Even if that conflict would be solved, will that ever get merged in? I'm with Oliver (see comments there) on the proposed change, there seems to be no real use case to not preserve attributes, so the options seem unnecessary. Should be straightforward to rip that part out of the PR, resolve the conflicts, and resubmit for review.

          If the patch in question is done in the right place, then it seems to me that we are just using a function from ANT to copy the file. Isn't that the function that strips the permissions?

          I don't see other places that could mess with the file properties.

          Ovidiu-Florin Bogdan added a comment - If the patch in question is done in the right place, then it seems to me that we are just using a function from ANT to copy the file. Isn't that the function that strips the permissions? I don't see other places that could mess with the file properties.

          Matt Sicker added a comment -

          The simple approach to fixing this is to preserve posix file permissions by default along with mtimes. A umask option could also be added to allow for reverting to the old behavior (umask of 133). Whether or not attempting to preserve file and group ownership is worthwhile may be another story.

          Matt Sicker added a comment - The simple approach to fixing this is to preserve posix file permissions by default along with mtimes. A umask option could also be added to allow for reverting to the old behavior (umask of 133). Whether or not attempting to preserve file and group ownership is worthwhile may be another story.

          Matt Sicker added a comment -

          I should be more specific about the umask. The way file creation works by default generally means working with a directory umask of 022 and "file umask" (not really a thing) of 133. The difference, of course, is because an executable directory means that it can be listed, not executed.

          Matt Sicker added a comment - I should be more specific about the umask. The way file creation works by default generally means working with a directory umask of 022 and "file umask" (not really a thing) of 133. The difference, of course, is because an executable directory means that it can be listed, not executed.

          Matt Sicker added a comment -

          Also important to note: this bug seems to only affect local->local file copying. Any local->remote or remote->local file copying happens via tar archives which preserve file permissions and are already handled as such.

          Matt Sicker added a comment - Also important to note: this bug seems to only affect local->local file copying. Any local->remote or remote->local file copying happens via tar archives which preserve file permissions and are already handled as such.

          Matt Sicker added a comment -

          Added a PR.

          Matt Sicker added a comment - Added a PR.

          Code changed in jenkins
          User: Matt Sicker
          Path:
          core/src/main/java/hudson/FilePath.java
          core/src/main/java/hudson/model/ItemGroupMixIn.java
          core/src/test/java/hudson/FilePathTest.java
          http://jenkins-ci.org/commit/jenkins/e229f37dd921812e213e34913c94b632c6e54299
          Log:
          JENKINS-13128: Preserve copied file permissions and mtime (#3400)

          This fixes a bug where files copied locally do not preserve file permissions or last modification time.

          • Use IO utility methods for exception handling
          • Fix invalid path exception propagation
          • Revert hudson.Util

          *NOTE:* This service been marked for deprecation: https://developer.github.com/changes/2018-04-25-github-services-deprecation/

          Functionality will be removed from GitHub.com on January 31st, 2019.

          SCM/JIRA link daemon added a comment - Code changed in jenkins User: Matt Sicker Path: core/src/main/java/hudson/FilePath.java core/src/main/java/hudson/model/ItemGroupMixIn.java core/src/test/java/hudson/FilePathTest.java http://jenkins-ci.org/commit/jenkins/e229f37dd921812e213e34913c94b632c6e54299 Log: JENKINS-13128 : Preserve copied file permissions and mtime (#3400) JENKINS-13128 : Preserve copied file permissions and mtime This fixes a bug where files copied locally do not preserve file permissions or last modification time. Use IO utility methods for exception handling Fix invalid path exception propagation Revert hudson.Util * NOTE: * This service been marked for deprecation: https://developer.github.com/changes/2018-04-25-github-services-deprecation/ Functionality will be removed from GitHub.com on January 31st, 2019.

          Devin Nusbaum added a comment -

          Fixed in Jenkins 2.120.

          Devin Nusbaum added a comment - Fixed in Jenkins 2.120 .

            jvz Matt Sicker
            tarwn Eli Weinstock-Herman
            Votes:
            7 Vote for this issue
            Watchers:
            12 Start watching this issue

              Created:
              Updated:
              Resolved: