Skip to content

Conversation

@Kevin-CB
Copy link

I noticed that we have many incorrect examples of how to use Groovy string interpolation on the Pipeline Examples page.

To prevent spreading this common mistake, it would be better to correct it.

See this documentation for more context.


//build your gradle flavor, passes the current build number as a parameter to gradle
sh "./gradlew clean assemble${flavor}Debug -PBUILD_NUMBER=${env.BUILD_NUMBER}"
sh './gradlew clean assemble${flavor}Debug -PBUILD_NUMBER=${BUILD_NUMBER}'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

${flavor} isn't an environment variable (BRANCH_NAME is) so this won't work. Use withEnv.


stage 'Stage Upload To Fabric'
sh "./gradlew crashlyticsUploadDistribution${flavor}Debug -PBUILD_NUMBER=${env.BUILD_NUMBER}"
sh './gradlew crashlyticsUploadDistribution${flavor}Debug -PBUILD_NUMBER=${BUILD_NUMBER}'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As above.

icon_emoji: ":jenkins:"])
sh "curl -X POST --data-urlencode \'payload=${payload}\' ${slackURL}"
withEnv(["PAYLOAD=${payload}", "SLACK_URL=${slackURL}"]) {
sh 'curl -X POST --data-urlencode "payload=$PAYLOAD" $SLACK_URL'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
sh 'curl -X POST --data-urlencode "payload=$PAYLOAD" $SLACK_URL'
sh 'curl -X POST --data-urlencode "payload=$PAYLOAD" "$SLACK_URL"'

// Look, no output directory under the root!
// pwd() outputs the current directory Pipeline is running in.
sh "ls -la ${pwd()}"
// PWD outputs the current directory Pipeline is running in.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kind of, but not really? The result is the same, assuming you use a shell that supports it, but this won't demonstrate the pwd() Pipeline feature. The phrasing seems off for the case where we just use a shell built-in feature.

(Granted, both the existing Pipeline and this one are really artificial, neither $PWD nor pwd() are actually needed.)


bat """
setlocal enabledelayedexpansion
\"${tool 'MSBuild'}\" SolutionName.sln /p:Configuration=Release /p:Platform=\"Any CPU\" /p:ProductVersion=1.0.0.!BUILD_NUMBER!
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you test this?

Even if it works, I'm unsure whether we should do it like this, as it doesn't provide an example of safe Pipeline authoring. There's still the double quote.

Comment on lines +7 to +11
bat """
setlocal enabledelayedexpansion
\"${tool 'MSBuild'}\" SolutionName.sln /p:Configuration=Release /p:Platform=\"Any CPU\" /p:ProductVersion=1.0.0.!BUILD_NUMBER!
endlocal
"""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps something like

Suggested change
bat """
setlocal enabledelayedexpansion
\"${tool 'MSBuild'}\" SolutionName.sln /p:Configuration=Release /p:Platform=\"Any CPU\" /p:ProductVersion=1.0.0.!BUILD_NUMBER!
endlocal
"""
def msbuild = tool 'MSBuild'
bat msbuild + " SolutionName.sln /p:Configuration=Release /p:Platform="Any CPU" /p:ProductVersion=1.0.0.%BUILD_NUMBER%"

Granted, this theoretically still has the problem described in https://plugins.jenkins.io/safe-batch-environment-filter/ but we know that %BUILD_NUMBER% is numeric. Perhaps mention it and move on?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants