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

Perforce Plugin Substitutes default Parameter Value instead of actual Parameter Value for client name

    XMLWordPrintable

Details

    • Bug
    • Status: Resolved (View Workflow)
    • Blocker
    • Resolution: Fixed
    • p4-plugin
    • perforce-plugin 1.3.28

    Description

      Assume a project has a parameter called "paramA" with a default value of "defaultValue". Also assume the client name format is set to ${nodename}_${paramA}. If the project is build with the parameter definition "paramA=otherValue" the effective client name resolved will still be <nodename>_defaultValue rather than <nodename>_otherValue as expected. This is because the default project substitutions are made first.

      Attachments

        Issue Links

          Activity

            rpetti Rob Petti added a comment -

            This is because the default project substitutions are made first.

            Can you please explain this statement? Other parameter substitutions work fine, and use the exact same process for performing replacements.

            rpetti Rob Petti added a comment - This is because the default project substitutions are made first. Can you please explain this statement? Other parameter substitutions work fine, and use the exact same process for performing replacements.
            stuartrowe Stuart Rowe added a comment -

            So this as of commit 2da2bf91851f3593a56c7d4a9408ec15e5d8aa79 as far as I can tell

            During SCM checkout, when getting the perforce workspace, the first step is to get the effective client name.

            1) Try to get the effective client name using the current project and build node (see call stack below)

            hudson.plugins.perforce.PerforceSCM.getEffectiveClientName(PerforceSCM.java:1702)
            hudson.plugins.perforce.PerforceSCM.getPerforceWorkspace(PerforceSCM.java:1581)
            hudson.plugins.perforce.PerforceSCM.getPerforceWorkspace(PerforceSCM.java:1569)
            hudson.plugins.perforce.PerforceSCM.checkout(PerforceSCM.java:867)
            hudson.model.AbstractProject.checkout(AbstractProject.java:1024)
            hudson.model.AbstractBuild$AbstractRunner.checkout(AbstractBuild.java:479)
            hudson.model.AbstractBuild$AbstractRunner.run(AbstractBuild.java:411)
            hudson.model.Run.run(Run.java:1198)
            hudson.model.FreeStyleBuild.run(FreeStyleBuild.java:46)
            hudson.model.ResourceController.execute(ResourceController.java:88)
            hudson.model.Executor.run(Executor.java:122)

            Digging deeper, if on a remote build node, then substitute tokens in the slave client name format. The problem is that the parameter maps being used for substitution don't include the current build's parameters. The substitution map is gathering entries from a number of default sources, including a set of default substitutions for the project:

            hudson.plugins.perforce.utils.MacroStringHelper.getDefaultSubstitutions(MacroStringHelper.java:369)
            hudson.plugins.perforce.utils.MacroStringHelper.substituteParametersNoCheck(MacroStringHelper.java:244)
            hudson.plugins.perforce.utils.MacroStringHelper.substituteParameters(MacroStringHelper.java:110)
            hudson.plugins.perforce.PerforceSCM.getEffectiveClientName(PerforceSCM.java:1735)
            hudson.plugins.perforce.PerforceSCM.getEffectiveClientName(PerforceSCM.java:1702)
            hudson.plugins.perforce.PerforceSCM.getPerforceWorkspace(PerforceSCM.java:1581)
            hudson.plugins.perforce.PerforceSCM.getPerforceWorkspace(PerforceSCM.java:1569)
            hudson.plugins.perforce.PerforceSCM.checkout(PerforceSCM.java:867)
            hudson.model.AbstractProject.checkout(AbstractProject.java:1024)
            hudson.model.AbstractBuild$AbstractRunner.checkout(AbstractBuild.java:479)
            hudson.model.AbstractBuild$AbstractRunner.run(AbstractBuild.java:411)
            hudson.model.Run.run(Run.java:1198)
            hudson.model.FreeStyleBuild.run(FreeStyleBuild.java:46)
            hudson.model.ResourceController.execute(ResourceController.java:88)
            hudson.model.Executor.run(Executor.java:122)

            At this point, if the slave client name format contains a token referencing a build parameter, it will be substituted by the default value of that parameter. (e.g. my_node_name_${paramA} --> my_node_name_defaultValue, as I described above in the issue). The token is now replaced and can't be substituted correctly if the build provides a different value for that parameter.

            2) Finally attempt to substitute any remaining tokens using the build and current environment map. For the example I described, there's no tokens left to replace and the substitution returns:

            hudson.plugins.perforce.utils.MacroStringHelper.substituteParametersNoCheck(MacroStringHelper.java:266)
            hudson.plugins.perforce.utils.MacroStringHelper.substituteParameters(MacroStringHelper.java:148)
            hudson.plugins.perforce.PerforceSCM.getEffectiveClientName(PerforceSCM.java:1707)
            hudson.plugins.perforce.PerforceSCM.getPerforceWorkspace(PerforceSCM.java:1581)
            hudson.plugins.perforce.PerforceSCM.getPerforceWorkspace(PerforceSCM.java:1569)
            hudson.plugins.perforce.PerforceSCM.checkout(PerforceSCM.java:867)
            hudson.model.AbstractProject.checkout(AbstractProject.java:1024)
            hudson.model.AbstractBuild$AbstractRunner.checkout(AbstractBuild.java:479)
            hudson.model.AbstractBuild$AbstractRunner.run(AbstractBuild.java:411)
            hudson.model.Run.run(Run.java:1198)
            hudson.model.FreeStyleBuild.run(FreeStyleBuild.java:46)
            hudson.model.ResourceController.execute(ResourceController.java:88)
            hudson.model.Executor.run(Executor.java:122)

            stuartrowe Stuart Rowe added a comment - So this as of commit 2da2bf91851f3593a56c7d4a9408ec15e5d8aa79 as far as I can tell During SCM checkout, when getting the perforce workspace, the first step is to get the effective client name. 1) Try to get the effective client name using the current project and build node (see call stack below) hudson.plugins.perforce.PerforceSCM.getEffectiveClientName(PerforceSCM.java:1702) hudson.plugins.perforce.PerforceSCM.getPerforceWorkspace(PerforceSCM.java:1581) hudson.plugins.perforce.PerforceSCM.getPerforceWorkspace(PerforceSCM.java:1569) hudson.plugins.perforce.PerforceSCM.checkout(PerforceSCM.java:867) hudson.model.AbstractProject.checkout(AbstractProject.java:1024) hudson.model.AbstractBuild$AbstractRunner.checkout(AbstractBuild.java:479) hudson.model.AbstractBuild$AbstractRunner.run(AbstractBuild.java:411) hudson.model.Run.run(Run.java:1198) hudson.model.FreeStyleBuild.run(FreeStyleBuild.java:46) hudson.model.ResourceController.execute(ResourceController.java:88) hudson.model.Executor.run(Executor.java:122) Digging deeper, if on a remote build node, then substitute tokens in the slave client name format. The problem is that the parameter maps being used for substitution don't include the current build's parameters. The substitution map is gathering entries from a number of default sources, including a set of default substitutions for the project: hudson.plugins.perforce.utils.MacroStringHelper.getDefaultSubstitutions(MacroStringHelper.java:369) hudson.plugins.perforce.utils.MacroStringHelper.substituteParametersNoCheck(MacroStringHelper.java:244) hudson.plugins.perforce.utils.MacroStringHelper.substituteParameters(MacroStringHelper.java:110) hudson.plugins.perforce.PerforceSCM.getEffectiveClientName(PerforceSCM.java:1735) hudson.plugins.perforce.PerforceSCM.getEffectiveClientName(PerforceSCM.java:1702) hudson.plugins.perforce.PerforceSCM.getPerforceWorkspace(PerforceSCM.java:1581) hudson.plugins.perforce.PerforceSCM.getPerforceWorkspace(PerforceSCM.java:1569) hudson.plugins.perforce.PerforceSCM.checkout(PerforceSCM.java:867) hudson.model.AbstractProject.checkout(AbstractProject.java:1024) hudson.model.AbstractBuild$AbstractRunner.checkout(AbstractBuild.java:479) hudson.model.AbstractBuild$AbstractRunner.run(AbstractBuild.java:411) hudson.model.Run.run(Run.java:1198) hudson.model.FreeStyleBuild.run(FreeStyleBuild.java:46) hudson.model.ResourceController.execute(ResourceController.java:88) hudson.model.Executor.run(Executor.java:122) At this point, if the slave client name format contains a token referencing a build parameter, it will be substituted by the default value of that parameter. (e.g. my_node_name_${paramA} --> my_node_name_defaultValue, as I described above in the issue). The token is now replaced and can't be substituted correctly if the build provides a different value for that parameter. 2) Finally attempt to substitute any remaining tokens using the build and current environment map. For the example I described, there's no tokens left to replace and the substitution returns: hudson.plugins.perforce.utils.MacroStringHelper.substituteParametersNoCheck(MacroStringHelper.java:266) hudson.plugins.perforce.utils.MacroStringHelper.substituteParameters(MacroStringHelper.java:148) hudson.plugins.perforce.PerforceSCM.getEffectiveClientName(PerforceSCM.java:1707) hudson.plugins.perforce.PerforceSCM.getPerforceWorkspace(PerforceSCM.java:1581) hudson.plugins.perforce.PerforceSCM.getPerforceWorkspace(PerforceSCM.java:1569) hudson.plugins.perforce.PerforceSCM.checkout(PerforceSCM.java:867) hudson.model.AbstractProject.checkout(AbstractProject.java:1024) hudson.model.AbstractBuild$AbstractRunner.checkout(AbstractBuild.java:479) hudson.model.AbstractBuild$AbstractRunner.run(AbstractBuild.java:411) hudson.model.Run.run(Run.java:1198) hudson.model.FreeStyleBuild.run(FreeStyleBuild.java:46) hudson.model.ResourceController.execute(ResourceController.java:88) hudson.model.Executor.run(Executor.java:122)
            rpetti Rob Petti added a comment -

            Looks like the order just needs to be changed, then? Move line 1707 above 1702. Unfortunately, I do not have a development environment available to make this change and test it.

            rpetti Rob Petti added a comment - Looks like the order just needs to be changed, then? Move line 1707 above 1702. Unfortunately, I do not have a development environment available to make this change and test it.
            stuartrowe Stuart Rowe added a comment -

            That was my first guess too -

            hudson.plugins.perforce.utils.MacroStringHelper.getDefaultSubstitutions(MacroStringHelper.java:369)
            hudson.plugins.perforce.utils.MacroStringHelper.substituteParametersNoCheck(MacroStringHelper.java:244)
            hudson.plugins.perforce.utils.MacroStringHelper.substituteParameters(MacroStringHelper.java:110)
            hudson.plugins.perforce.PerforceSCM.getEffectiveClientName(PerforceSCM.java:1735) <-- but don't get the the slave name format until here
            hudson.plugins.perforce.PerforceSCM.getEffectiveClientName(PerforceSCM.java:1702) <-- order would need to change here
            hudson.plugins.perforce.PerforceSCM.getPerforceWorkspace(PerforceSCM.java:1581)
            hudson.plugins.perforce.PerforceSCM.getPerforceWorkspace(PerforceSCM.java:1569)
            hudson.plugins.perforce.PerforceSCM.checkout(PerforceSCM.java:867)
            hudson.model.AbstractProject.checkout(AbstractProject.java:1024)
            hudson.model.AbstractBuild$AbstractRunner.checkout(AbstractBuild.java:479)
            hudson.model.AbstractBuild$AbstractRunner.run(AbstractBuild.java:411)
            hudson.model.Run.run(Run.java:1198)
            hudson.model.FreeStyleBuild.run(FreeStyleBuild.java:46)
            hudson.model.ResourceController.execute(ResourceController.java:88)
            hudson.model.Executor.run(Executor.java:122)

            I think the current build variables need to be included in the substitutions map AFTER the default values are added when substituting parameters on the client name. I can look into a potential fix tomorrow.

            stuartrowe Stuart Rowe added a comment - That was my first guess too - hudson.plugins.perforce.utils.MacroStringHelper.getDefaultSubstitutions(MacroStringHelper.java:369) hudson.plugins.perforce.utils.MacroStringHelper.substituteParametersNoCheck(MacroStringHelper.java:244) hudson.plugins.perforce.utils.MacroStringHelper.substituteParameters(MacroStringHelper.java:110) hudson.plugins.perforce.PerforceSCM.getEffectiveClientName(PerforceSCM.java:1735) <-- but don't get the the slave name format until here hudson.plugins.perforce.PerforceSCM.getEffectiveClientName(PerforceSCM.java:1702) <-- order would need to change here hudson.plugins.perforce.PerforceSCM.getPerforceWorkspace(PerforceSCM.java:1581) hudson.plugins.perforce.PerforceSCM.getPerforceWorkspace(PerforceSCM.java:1569) hudson.plugins.perforce.PerforceSCM.checkout(PerforceSCM.java:867) hudson.model.AbstractProject.checkout(AbstractProject.java:1024) hudson.model.AbstractBuild$AbstractRunner.checkout(AbstractBuild.java:479) hudson.model.AbstractBuild$AbstractRunner.run(AbstractBuild.java:411) hudson.model.Run.run(Run.java:1198) hudson.model.FreeStyleBuild.run(FreeStyleBuild.java:46) hudson.model.ResourceController.execute(ResourceController.java:88) hudson.model.Executor.run(Executor.java:122) I think the current build variables need to be included in the substitutions map AFTER the default values are added when substituting parameters on the client name. I can look into a potential fix tomorrow.
            oleg_nenashev Oleg Nenashev added a comment -

            This issue has been caused by a new variables substitution mechanism.
            https://github.com/jenkinsci/perforce-plugin/pull/54

            I'll handle it

            oleg_nenashev Oleg Nenashev added a comment - This issue has been caused by a new variables substitution mechanism. https://github.com/jenkinsci/perforce-plugin/pull/54 I'll handle it
            stuartrowe Stuart Rowe added a comment -

            Thanks, I never got around to looking into it

            stuartrowe Stuart Rowe added a comment - Thanks, I never got around to looking into it
            oleg_nenashev Oleg Nenashev added a comment -

            Caused by JENKINS-23467 in 1.3.28.
            The issue seems to be a blocker, a noting on Wiki is required

            oleg_nenashev Oleg Nenashev added a comment - Caused by JENKINS-23467 in 1.3.28. The issue seems to be a blocker, a noting on Wiki is required
            oleg_nenashev Oleg Nenashev added a comment - https://github.com/jenkinsci/perforce-plugin/pull/59

            Code changed in jenkins
            User: Oleg Nenashev
            Path:
            src/test/java/hudson/plugins/perforce/PerforceSCMTest.java
            http://jenkins-ci.org/commit/perforce-plugin/7fdf8d0d580661cf12c15f054462f15d30cd61b2
            Log:
            JENKINS-25226] - A direct unit test for the issue

            The test checks that the variables substitution works properly for build parameters.
            The change also refactors the provisioning of PerforceSCM stubs.

            Signed-off-by: Oleg Nenashev <o.v.nenashev@gmail.com>

            scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Oleg Nenashev Path: src/test/java/hudson/plugins/perforce/PerforceSCMTest.java http://jenkins-ci.org/commit/perforce-plugin/7fdf8d0d580661cf12c15f054462f15d30cd61b2 Log: JENKINS-25226 ] - A direct unit test for the issue The test checks that the variables substitution works properly for build parameters. The change also refactors the provisioning of PerforceSCM stubs. Signed-off-by: Oleg Nenashev <o.v.nenashev@gmail.com>

            Code changed in jenkins
            User: Oleg Nenashev
            Path:
            src/main/java/hudson/plugins/perforce/utils/MacroStringHelper.java
            http://jenkins-ci.org/commit/perforce-plugin/7b22e0b18e27941d07269d53b63e3720841944e5
            Log:
            [FIXED JENKINS-25226] - Resolve environment variables with a highest priority

            Other variables (projects, default parameters, etc.) will be used in the case of missing macros.
            It should not happen in general, but may appear in buildEnvironment()

            Signed-off-by: Oleg Nenashev <o.v.nenashev@gmail.com>

            scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Oleg Nenashev Path: src/main/java/hudson/plugins/perforce/utils/MacroStringHelper.java http://jenkins-ci.org/commit/perforce-plugin/7b22e0b18e27941d07269d53b63e3720841944e5 Log: [FIXED JENKINS-25226] - Resolve environment variables with a highest priority Other variables (projects, default parameters, etc.) will be used in the case of missing macros. It should not happen in general, but may appear in buildEnvironment() Signed-off-by: Oleg Nenashev <o.v.nenashev@gmail.com>

            Code changed in jenkins
            User: Rob Petti
            Path:
            src/main/java/hudson/plugins/perforce/utils/MacroStringHelper.java
            src/test/java/com/synopsys/arc/jenkinsci/plugins/perforce/MacroStringHelperTest.java
            src/test/java/hudson/plugins/perforce/PerforceSCMTest.java
            http://jenkins-ci.org/commit/perforce-plugin/fe2717138270aa9a927730dfc7b91e1cc9a97495
            Log:
            Merge pull request #59 from synopsys-arc-oss/Variables_handling_issues

            [FIXED JENKINS-25635,JENKINS-25226] - Variables substitution issues

            Compare: https://github.com/jenkinsci/perforce-plugin/compare/44c10b2c639c...fe2717138270

            scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Rob Petti Path: src/main/java/hudson/plugins/perforce/utils/MacroStringHelper.java src/test/java/com/synopsys/arc/jenkinsci/plugins/perforce/MacroStringHelperTest.java src/test/java/hudson/plugins/perforce/PerforceSCMTest.java http://jenkins-ci.org/commit/perforce-plugin/fe2717138270aa9a927730dfc7b91e1cc9a97495 Log: Merge pull request #59 from synopsys-arc-oss/Variables_handling_issues [FIXED JENKINS-25635,JENKINS-25226] - Variables substitution issues Compare: https://github.com/jenkinsci/perforce-plugin/compare/44c10b2c639c...fe2717138270
            oleg_nenashev Oleg Nenashev added a comment -

            Stuart reported that the issue has not been solved for client names.
            Reopening the issue

            oleg_nenashev Oleg Nenashev added a comment - Stuart reported that the issue has not been solved for client names. Reopening the issue
            oleg_nenashev Oleg Nenashev added a comment -

            Created JENKINS-25559 for Stuart's case

            oleg_nenashev Oleg Nenashev added a comment - Created JENKINS-25559 for Stuart's case
            bscriver Brent Scriver added a comment -

            I have seen this issue in 1.3.29 of the Perforce plugin where the workspace field was set to ${CLIENTSPEC} and the default value for ${CLIENTSPEC} was empty and is populated by incoming parameters from triggering jobs. The client name format for slaves was just ${basename}. 1.3.29 was selecting the default value instead of the incoming value. Downgrading to 1.3.27 addressed my issue.

            bscriver Brent Scriver added a comment - I have seen this issue in 1.3.29 of the Perforce plugin where the workspace field was set to ${CLIENTSPEC} and the default value for ${CLIENTSPEC} was empty and is populated by incoming parameters from triggering jobs. The client name format for slaves was just ${basename}. 1.3.29 was selecting the default value instead of the incoming value. Downgrading to 1.3.27 addressed my issue.

            People

              oleg_nenashev Oleg Nenashev
              stuartrowe Stuart Rowe
              Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved: