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

AbstractBuild.getEnvironment does not set EnvVars.platform and breaks EnvVars.override

    XMLWordPrintable

    Details

    • Similar Issues:

      Description

      Hi,
      Since 1.472, calling AbstractBuild.getEnvironment(TaskListener) returns a EnvVars with EnvVars.platform unset.
      Since EnvVars.override uses EnvVars.platform (and default to UNIX style PATH separator if EnvVars.platform is null), it does not work anymore for Windows slaves.
      Thanks!

        Attachments

          Issue Links

            Activity

            Hide
            nfalco Nikolas Falco added a comment - - edited

            This 2012 bug hit again (nodejs plugin).
            It break all SimpleBuildWrapper that override PATH+VAR launched on node with different platform (Unix->Windows and vs).
            The issue seems to be in Run#getEnvironment(TaskListener) that returns an enviroment EnvVars not built from Computer class (the unique way to have platform field setup correctly)

            Run.java
                 * Unlike earlier {@link #getEnvVars()}, this map contains the whole environment,
                 * not just the overrides, so one can introspect values to change its behavior.
                 * ...
                 */
                public @Nonnull EnvVars getEnvironment(@Nonnull TaskListener listener) throws IOException, InterruptedException {
                    Computer c = Computer.currentComputer();
                    Node n = c==null ? null : c.getNode();
            
                    EnvVars env = getParent().getEnvironment(n,listener);
                    env.putAll(getCharacteristicEnvVars());
                    ...
                }
            

            I think that something similar fix the issue

            Run.java
                public @Nonnull EnvVars getEnvironment(@Nonnull TaskListener listener) throws IOException, InterruptedException {
                    Computer c = Computer.currentComputer();
                    Node n = c==null ? null : c.getNode();
            
                    EnvVars env = c.getEnvironment();
                    env.putAll(getParent().getEnvironment(n,listener));
                    env.putAll(getCharacteristicEnvVars());
                    ...
                }
            

            untill a fix is released my dirty workaround is create a EnvironmentContributor that by reflection inject correct platform value

            FixEnvVarEnvironmentContributor.java
            @Extension(ordinal = -200)
            public class FixEnvVarEnvironmentContributor extends EnvironmentContributor {
            
                @Override
                public void buildEnvironmentFor(@SuppressWarnings("rawtypes") @Nonnull Run run, @Nonnull EnvVars envs, @Nonnull TaskListener listener) throws IOException, InterruptedException {
                    Computer c = Computer.currentComputer();
                    if (c != null) {
                        Field platformField = ReflectionUtils.findField(EnvVars.class, "platform", Platform.class);
                        ReflectionUtils.makeAccessible(platformField);
                        Platform currentPlatform = (Platform) ReflectionUtils.getField(platformField, envs);
                        if (currentPlatform == null) {
                            // try to fix value with than one that comes from current computer
                            EnvVars remoteEnv = c.getEnvironment();
                            Platform computerPlatform = (Platform) ReflectionUtils.getField(platformField, remoteEnv);
                            if (computerPlatform != null) {
                                ReflectionUtils.setField(platformField, envs, computerPlatform);
                            }
                        }
                    }
                }
            }
            
            Show
            nfalco Nikolas Falco added a comment - - edited This 2012 bug hit again (nodejs plugin). It break all SimpleBuildWrapper that override PATH+VAR launched on node with different platform (Unix->Windows and vs). The issue seems to be in Run#getEnvironment(TaskListener) that returns an enviroment EnvVars not built from Computer class (the unique way to have platform field setup correctly) Run.java * Unlike earlier {@link #getEnvVars()}, this map contains the whole environment, * not just the overrides, so one can introspect values to change its behavior. * ... */ public @Nonnull EnvVars getEnvironment(@Nonnull TaskListener listener) throws IOException, InterruptedException { Computer c = Computer.currentComputer(); Node n = c== null ? null : c.getNode(); EnvVars env = getParent().getEnvironment(n,listener); env.putAll(getCharacteristicEnvVars()); ... } I think that something similar fix the issue Run.java public @Nonnull EnvVars getEnvironment(@Nonnull TaskListener listener) throws IOException, InterruptedException { Computer c = Computer.currentComputer(); Node n = c== null ? null : c.getNode(); EnvVars env = c.getEnvironment(); env.putAll(getParent().getEnvironment(n,listener)); env.putAll(getCharacteristicEnvVars()); ... } untill a fix is released my dirty workaround is create a EnvironmentContributor that by reflection inject correct platform value FixEnvVarEnvironmentContributor.java @Extension(ordinal = -200) public class FixEnvVarEnvironmentContributor extends EnvironmentContributor { @Override public void buildEnvironmentFor(@SuppressWarnings( "rawtypes" ) @Nonnull Run run, @Nonnull EnvVars envs, @Nonnull TaskListener listener) throws IOException, InterruptedException { Computer c = Computer.currentComputer(); if (c != null ) { Field platformField = ReflectionUtils.findField(EnvVars.class, "platform" , Platform.class); ReflectionUtils.makeAccessible(platformField); Platform currentPlatform = (Platform) ReflectionUtils.getField(platformField, envs); if (currentPlatform == null ) { // try to fix value with than one that comes from current computer EnvVars remoteEnv = c.getEnvironment(); Platform computerPlatform = (Platform) ReflectionUtils.getField(platformField, remoteEnv); if (computerPlatform != null ) { ReflectionUtils.setField(platformField, envs, computerPlatform); } } } } }
            Hide
            scm_issue_link SCM/JIRA link daemon added a comment -

            Code changed in jenkins
            User: Nikolas Falco
            Path:
            src/main/java/jenkins/plugins/nodejs/tools/pathresolvers/FixEnvVarEnvironmentContributor.java
            http://jenkins-ci.org/commit/nodejs-plugin/dfefe0c16776c41a849894f8b738d52fcbb8a4ba
            Log:
            Fix for JENKINS-14807

            Compare: https://github.com/jenkinsci/nodejs-plugin/compare/1eeb228b9500...dfefe0c16776

            Show
            scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Nikolas Falco Path: src/main/java/jenkins/plugins/nodejs/tools/pathresolvers/FixEnvVarEnvironmentContributor.java http://jenkins-ci.org/commit/nodejs-plugin/dfefe0c16776c41a849894f8b738d52fcbb8a4ba Log: Fix for JENKINS-14807 Compare: https://github.com/jenkinsci/nodejs-plugin/compare/1eeb228b9500...dfefe0c16776
            Hide
            scm_issue_link SCM/JIRA link daemon added a comment -

            Code changed in jenkins
            User: Nikolas Falco
            Path:
            src/main/java/jenkins/plugins/nodejs/tools/pathresolvers/FixEnvVarEnvironmentContributor.java
            http://jenkins-ci.org/commit/nodejs-plugin/89ec9ee9777b821a65e83f4220e0821b02606fd6
            Log:
            Workaround for JENKINS-14807

            Show
            scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Nikolas Falco Path: src/main/java/jenkins/plugins/nodejs/tools/pathresolvers/FixEnvVarEnvironmentContributor.java http://jenkins-ci.org/commit/nodejs-plugin/89ec9ee9777b821a65e83f4220e0821b02606fd6 Log: Workaround for JENKINS-14807
            Hide
            nfalco Nikolas Falco added a comment -

            There is a PR here with a unit test

            Show
            nfalco Nikolas Falco added a comment - There is a PR here with a unit test
            Hide
            scm_issue_link SCM/JIRA link daemon added a comment -

            Code changed in jenkins
            User: Nikolas Falco
            Path:
            core/src/main/java/hudson/model/Job.java
            core/src/test/java/hudson/model/JobTest.java
            http://jenkins-ci.org/commit/jenkins/cf0183d1ed1e999a04a1445b2cd369b58e1268bf
            Log:
            JENKINS-14807 Fix path separator when EnvVars overrides variable like
            PATH+XYZ

            The getEnvironment(Node, TaskListener) now returns an environment setup
            correctly with the platform value of the computer node where the job is
            executed.

            Show
            scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Nikolas Falco Path: core/src/main/java/hudson/model/Job.java core/src/test/java/hudson/model/JobTest.java http://jenkins-ci.org/commit/jenkins/cf0183d1ed1e999a04a1445b2cd369b58e1268bf Log: JENKINS-14807 Fix path separator when EnvVars overrides variable like PATH+XYZ The getEnvironment(Node, TaskListener) now returns an environment setup correctly with the platform value of the computer node where the job is executed.
            Hide
            scm_issue_link SCM/JIRA link daemon added a comment -

            Code changed in jenkins
            User: Oleg Nenashev
            Path:
            core/src/main/java/hudson/model/Job.java
            core/src/test/java/hudson/model/JobTest.java
            http://jenkins-ci.org/commit/jenkins/6595ec34d05fe7fa170dd1481a3fb86a85796852
            Log:
            Merge pull request #2936 from nfalco79/feature/JENKINS-14807

            JENKINS-14807 Fix path separator when EnvVars overrides variable like PATH+XYZ on slave node cross platform

            Compare: https://github.com/jenkinsci/jenkins/compare/19f2d66334cb...6595ec34d05f

            Show
            scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Oleg Nenashev Path: core/src/main/java/hudson/model/Job.java core/src/test/java/hudson/model/JobTest.java http://jenkins-ci.org/commit/jenkins/6595ec34d05fe7fa170dd1481a3fb86a85796852 Log: Merge pull request #2936 from nfalco79/feature/ JENKINS-14807 JENKINS-14807 Fix path separator when EnvVars overrides variable like PATH+XYZ on slave node cross platform Compare: https://github.com/jenkinsci/jenkins/compare/19f2d66334cb...6595ec34d05f
            Hide
            oleg_nenashev Oleg Nenashev added a comment -

            It has been released in 2.71

            Show
            oleg_nenashev Oleg Nenashev added a comment - It has been released in 2.71
            Hide
            scm_issue_link SCM/JIRA link daemon added a comment -

            Code changed in jenkins
            User: Nikolas Falco
            Path:
            src/main/java/jenkins/plugins/nodejs/tools/pathresolvers/FixEnvVarEnvironmentContributor.java
            http://jenkins-ci.org/commit/nodejs-plugin/aecf8fe2f12a7c98f978c9fd1fd28cfb5cbc08a2
            Log:
            Remove JENKINS-14807 patch because released in Jenkins 2.71 and in 2.60.3 LTS

            Show
            scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Nikolas Falco Path: src/main/java/jenkins/plugins/nodejs/tools/pathresolvers/FixEnvVarEnvironmentContributor.java http://jenkins-ci.org/commit/nodejs-plugin/aecf8fe2f12a7c98f978c9fd1fd28cfb5cbc08a2 Log: Remove JENKINS-14807 patch because released in Jenkins 2.71 and in 2.60.3 LTS

              People

              • Assignee:
                Unassigned
                Reporter:
                mansion Olivier Mansion
              • Votes:
                2 Vote for this issue
                Watchers:
                7 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: