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

Missing hash in client names if ClientNameFormat substitution fails

      This bug appears in the following conditions:

      • Client name contains ${EXECUTOR_NUMBER} variable

      High-priority issue appears if:

      • Concurrent builds are enabled
      • No NODE_NAME variable in client name
        => Missing hash in Client name => collisions in workspaces on different nodes

          [JENKINS-25559] Missing hash in client names if ClientNameFormat substitution fails

          Oleg Nenashev added a comment -

          Caused by JENKINS-23467

          Oleg Nenashev added a comment - Caused by JENKINS-23467

          Oleg Nenashev added a comment -

          Oleg Nenashev added a comment - Fixed by stuartrowe in https://github.com/jenkinsci/perforce-plugin/pull/60

          Code changed in jenkins
          User: Oleg Nenashev
          Path:
          src/main/java/hudson/plugins/perforce/PerforceSCM.java
          http://jenkins-ci.org/commit/perforce-plugin/b3c84d1f4db2175273dce62652fea041e8ae2ac5
          Log:
          Merge pull request #60 from stuartrowe/master

          [FIXED JENKINS-25559] - Don't rename remote clients in concurrent builds

          Compare: https://github.com/jenkinsci/perforce-plugin/compare/71dd14e84751...b3c84d1f4db2

          SCM/JIRA link daemon added a comment - Code changed in jenkins User: Oleg Nenashev Path: src/main/java/hudson/plugins/perforce/PerforceSCM.java http://jenkins-ci.org/commit/perforce-plugin/b3c84d1f4db2175273dce62652fea041e8ae2ac5 Log: Merge pull request #60 from stuartrowe/master [FIXED JENKINS-25559] - Don't rename remote clients in concurrent builds Compare: https://github.com/jenkinsci/perforce-plugin/compare/71dd14e84751...b3c84d1f4db2

          Oleg Nenashev added a comment -

          The fix does not resolve the issue
          New PR: https://github.com/jenkinsci/perforce-plugin/pull/61

          Seems MacroSubstitutionHelper should catch recursion calls to prevent the issue

          Oleg Nenashev added a comment - The fix does not resolve the issue New PR: https://github.com/jenkinsci/perforce-plugin/pull/61 Seems MacroSubstitutionHelper should catch recursion calls to prevent the issue

          Rob Petti added a comment -

          This is a configuration issue, isn't it? If you don't have the nodename, hostname, or hash in the client name for slaves option, then you will get collisions.

          Rob Petti added a comment - This is a configuration issue, isn't it? If you don't have the nodename, hostname, or hash in the client name for slaves option, then you will get collisions.

          Oleg Nenashev added a comment - - edited

          If PerforceSCM fails to resolve the client name (it happens due to unresolved EXECUTOR_NUMBER), it fall-backs to a default value without a hash.
          Currently, I'm working on this issue. The proper way would be to limit the number of nested recursions in buildEnvVars() by 2 instead of 1 (current implementation)

          private String getEffectiveClientName(@Nonnull AbstractBuild build, @CheckForNull Map<String,String> env) 
                      throws ParameterSubstitutionException, InterruptedException {
                  Node buildNode = build.getBuiltOn();
                  FilePath workspace = build.getWorkspace();
                  String effectiveP4Client = this.p4Client;
                  try {
                      effectiveP4Client = getEffectiveClientName(effectiveP4Client, build);
                  } catch (Exception e) {
                      new StreamTaskListener(System.out).getLogger().println(
                              "Could not get effective client name: " + e.getMessage());
                  }
                  effectiveP4Client = MacroStringHelper.substituteParameters(effectiveP4Client, this, build, env);
                  return effectiveP4Client;
              }
          

          Oleg Nenashev added a comment - - edited If PerforceSCM fails to resolve the client name (it happens due to unresolved EXECUTOR_NUMBER), it fall-backs to a default value without a hash. Currently, I'm working on this issue. The proper way would be to limit the number of nested recursions in buildEnvVars() by 2 instead of 1 (current implementation) private String getEffectiveClientName(@Nonnull AbstractBuild build, @CheckForNull Map<String,String> env) throws ParameterSubstitutionException, InterruptedException { Node buildNode = build.getBuiltOn(); FilePath workspace = build.getWorkspace(); String effectiveP4Client = this.p4Client; try { effectiveP4Client = getEffectiveClientName(effectiveP4Client, build); } catch (Exception e) { new StreamTaskListener(System.out).getLogger().println( "Could not get effective client name: " + e.getMessage()); } effectiveP4Client = MacroStringHelper.substituteParameters(effectiveP4Client, this, build, env); return effectiveP4Client; }

          Oleg Nenashev added a comment -

          rpetti
          IMO it would be preferable to completely prohibit runtime variables in client names, but it will break many use-cases.
          I'll try to find a convenient solution

          Oleg Nenashev added a comment - rpetti IMO it would be preferable to completely prohibit runtime variables in client names, but it will break many use-cases. I'll try to find a convenient solution

          Oleg Nenashev added a comment -

          The issue has been fixed in PR #60 and PR #61
          https://github.com/jenkinsci/perforce-plugin/pull/61
          https://github.com/jenkinsci/perforce-plugin/pull/60

          Automatic SCM Linking does not work

          Oleg Nenashev added a comment - The issue has been fixed in PR #60 and PR #61 https://github.com/jenkinsci/perforce-plugin/pull/61 https://github.com/jenkinsci/perforce-plugin/pull/60 Automatic SCM Linking does not work

          Oleg Nenashev added a comment -

          1.3.31 should resolve the issue.
          // SCM Link bot seems to be dead

          Oleg Nenashev added a comment - 1.3.31 should resolve the issue. // SCM Link bot seems to be dead

            oleg_nenashev Oleg Nenashev
            oleg_nenashev Oleg Nenashev
            Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

              Created:
              Updated:
              Resolved: