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

Check variable names as Jenkins core expands

    XMLWordPrintable

Details

    Description

      After the parameterized trigger plugin wants to exchange all choices to extensible choices we have to change many properties in our jobs. But that's OK.

      Problem:
      Because of many of our properties containing dots we have to overwork half of our build process.

      Suggestion:
      Please check if the name of a property can contain . too.


      Extensible Choice Parameter checks parameter names as Jenkins 1.466 expands variable
      (that is, alphabets, numbers, and underscore: https://github.com/jenkinsci/jenkins/blob/jenkins-1.466/core/src/main/java/hudson/Util.java#L111 )

      Jenkins 1.526 gets to allow dots in JENKINS-16660, f664639.

      Extensible Choice Parameter should check parameter names as the Jenkins core it runs on.

      Attachments

        Issue Links

          Activity

            zioschild Sven Appenrodt created issue -

            Hint: getting triggerd by the parameterized trigger plugin whith having a choice "foo.bar" works, only the validation check for this field fails. Didn't tested accessing the choice via e.g. groovy...

            zioschild Sven Appenrodt added a comment - Hint: getting triggerd by the parameterized trigger plugin whith having a choice "foo.bar" works, only the validation check for this field fails. Didn't tested accessing the choice via e.g. groovy...
            ikedam ikedam added a comment -

            It depends on how to use it.
            If used in shell or windows batch, it would does not work correct.
            Current parameter name check is performed under the rule of bash (alphabets, numbers, underscore).

            Though I haven't tested yet, it would work correct in almost other cases even the configuration page displays an error. Groovy scripts would also work well as they access build variables as a map object.

            I'll ask Jenkins developers whether there is a guideline about parameter names, and consider what's the best way.

            For now, I think followings are good for your case.

            • Test parameters with dots work correct in your environments. Error messages in configuration pages are just informational, and Extensible Choice Parameter does not kick those values even in future releases.
              • But I cannot ensure Jenkins itself accepts any values even in future reases...
            • I still recommend you to use bash-like parameter names if you can easily adapt it. There may be a day you want to access those values from bash scripts or windows batches.
            ikedam ikedam added a comment - It depends on how to use it. If used in shell or windows batch, it would does not work correct. Current parameter name check is performed under the rule of bash (alphabets, numbers, underscore). Though I haven't tested yet, it would work correct in almost other cases even the configuration page displays an error. Groovy scripts would also work well as they access build variables as a map object. I'll ask Jenkins developers whether there is a guideline about parameter names, and consider what's the best way. For now, I think followings are good for your case. Test parameters with dots work correct in your environments. Error messages in configuration pages are just informational, and Extensible Choice Parameter does not kick those values even in future releases. But I cannot ensure Jenkins itself accepts any values even in future reases... I still recommend you to use bash-like parameter names if you can easily adapt it. There may be a day you want to access those values from bash scripts or windows batches.

            Thanks for asking.
            I assume there is no guideline but asking doesn't cost anything than a little time

            In that case - can we make a compromise that you continue checking the names but display the result (if any) as warning? Maybe many users will missinterpret your "error", but a warning like "use bash like names [a-zA-Z_-], there might be problems with using this parameter in actions" or something like this might be better. It's just to warn the user using that kind of name, not to forbid it.

            Thanks a lot

            zioschild Sven Appenrodt added a comment - Thanks for asking. I assume there is no guideline but asking doesn't cost anything than a little time In that case - can we make a compromise that you continue checking the names but display the result (if any) as warning? Maybe many users will missinterpret your "error", but a warning like "use bash like names [a-zA-Z_-] , there might be problems with using this parameter in actions" or something like this might be better. It's just to warn the user using that kind of name, not to forbid it. Thanks a lot
            ikedam ikedam added a comment -

            I'm not sure now. I know I can take one of follewing ways:

            • Not check names at all.
            • Check, but dispay only as a warning. (as your suggest)
            • Check and display as a error. But never forbid it actually (as current implemantation).

            Let me have time to consider which is the best way.

            ikedam ikedam added a comment - I'm not sure now. I know I can take one of follewing ways: Not check names at all. Check, but dispay only as a warning. (as your suggest) Check and display as a error. But never forbid it actually (as current implemantation). Let me have time to consider which is the best way.
            ikedam ikedam added a comment -

            https://github.com/jenkinsci/jenkins/blob/master/core/src/main/java/hudson/Util.java#L116

            /**
             * Pattern for capturing variables. Either $xyz, ${xyz} or ${a.b} but not $a.b, while ignoring "$$"
             */
            private static final Pattern VARIABLE = Pattern.compile("\\$([A-Za-z0-9_]+|\\{[A-Za-z0-9_.]+\\}|\\$)");
            

            This seems available from jenkins-1.526 (JENKINS-16660). Dots are not accepted before.

            ikedam ikedam added a comment - https://github.com/jenkinsci/jenkins/blob/master/core/src/main/java/hudson/Util.java#L116 /** * Pattern for capturing variables. Either $xyz, ${xyz} or ${a.b} but not $a.b, while ignoring "$$" */ private static final Pattern VARIABLE = Pattern.compile( "\\$([A-Za-z0-9_]+|\\{[A-Za-z0-9_.]+\\}|\\$)" ); This seems available from jenkins-1.526 ( JENKINS-16660 ). Dots are not accepted before.
            ikedam ikedam made changes -
            Field Original Value New Value
            Link This issue is related to JENKINS-16660 [ JENKINS-16660 ]
            ikedam ikedam added a comment -

            I decided how to do.
            Target version is 1.3.0.

            ikedam ikedam added a comment - I decided how to do. Target version is 1.3.0.
            ikedam ikedam made changes -
            Assignee ikedam [ ikedam ]
            Description After the parameterized trigger plugin wants to exchange all choices to extensible choices we have to change many properties in our jobs. But that's OK.

            Problem:
            Because of many of our properties containing dots we have to overwork half of our build process.

            Suggestion:
            Please check if the name of a property can contain . too.
            After the parameterized trigger plugin wants to exchange all choices to extensible choices we have to change many properties in our jobs. But that's OK.

            Problem:
            Because of many of our properties containing dots we have to overwork half of our build process.

            Suggestion:
            Please check if the name of a property can contain . too.

            ----

            Extensible Choice Parameter checks parameter names as Jenkins 1.466 expands variable
            (that is, alphabets, numbers, and underscore: https://github.com/jenkinsci/jenkins/blob/jenkins-1.466/core/src/main/java/hudson/Util.java#L111 )

            Jenkins 1.526 gets to allow dots in JENKINS-16660, [f664639|https://github.com/jenkinsci/jenkins/commit/f664639b79d2dea2abdf48816b01f8ccc525a73f].

            Extensible Choice Parameter should check parameter names as the Jenkins core it runs on.
            Issue Type Task [ 3 ] Improvement [ 4 ]
            Priority Minor [ 4 ] Major [ 3 ]
            Summary Extend name pattern to use dots too Check variable names as Jenkins core expands
            ikedam ikedam added a comment - Fixed as https://github.com/jenkinsci/extensible-choice-parameter-plugin/pull/7
            ikedam ikedam added a comment -

            Really sorry for having you waiting for so long time.
            I released extensible-choice-parameter-1.3.0 including this fix.
            It will be available in a day.

            ikedam ikedam added a comment - Really sorry for having you waiting for so long time. I released extensible-choice-parameter-1.3.0 including this fix. It will be available in a day.
            ikedam ikedam made changes -
            Resolution Fixed [ 1 ]
            Status Open [ 1 ] Resolved [ 5 ]

            Works perfect. Thanks

            zioschild Sven Appenrodt added a comment - Works perfect. Thanks
            zioschild Sven Appenrodt made changes -
            Status Resolved [ 5 ] Closed [ 6 ]
            rtyler R. Tyler Croy made changes -
            Workflow JNJira [ 154302 ] JNJira + In-Review [ 207523 ]

            People

              ikedam ikedam
              zioschild Sven Appenrodt
              Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved: