osutil: add support for shell conditional syntax in envs - #15446
Merged
bboozzoo merged 6 commits intoJun 11, 2026
Merged
Conversation
sergio-costas
force-pushed
the
allow-conditional-environment-variables
branch
from
May 14, 2025 10:29
a07fee4 to
b0f184a
Compare
|
Wed Jun 10 00:48:40 UTC 2026 Failures:Preparing:
Executing:
Restoring:
Skipped tests from snapd-testing-skipIf you wish to have any of the below tests run in your PR, in your PR description, add 'unskip:' followed by a copy-and-pasted list (without variants) of the below tests you wish to run (unskip plus test list must be valid yaml)
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #15446 +/- ##
========================================
Coverage 79.10% 79.10%
========================================
Files 1384 1384
Lines 192855 192991 +136
Branches 2466 2466
========================================
+ Hits 152549 152672 +123
+ Misses 31138 31134 -4
- Partials 9168 9185 +17
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
sergio-costas
force-pushed
the
allow-conditional-environment-variables
branch
2 times, most recently
from
May 20, 2025 09:31
996085c to
b906b45
Compare
bboozzoo
reviewed
May 26, 2025
Contributor
There was a problem hiding this comment.
needs an update to the docs on the forum, does it work with snapcraft already?
Also IIRC this does not work "${FOO:-${BAR}}` and will need to be documented
Contributor
Author
There was a problem hiding this comment.
It needs an update in the docs, yes, because currently, putting that on an environment variable replaces it with an empty string. But I was waiting until a review for that.
Also, you are right, it won't work with ${FOO:-${BAR}}. I can do a full-fledged parser, but I think that it would be excessive, and that just by documenting that limitation should be enough.
sergio-costas
force-pushed
the
allow-conditional-environment-variables
branch
2 times, most recently
from
May 26, 2025 12:12
e281c28 to
9928edd
Compare
zyga
reviewed
May 28, 2025
zyga
left a comment
zyga
left a comment
Contributor
There was a problem hiding this comment.
Is the semantic to expand that in snap-exec?
Is there a specific use case you are after?
pedronis
added
⛔ Blocked
Needs Samuele review
Contributor
Author
|
@zyga I wanted to add some extra paths in https://github.com/canonical/ubuntu-desktop-session-snap/pull/39/files , and tried that syntax, but at the end had to add all the required paths, and was James who pointed out that it won't work as I expected. |
Contributor
Author
|
@zyga more details about why: the point is that, in some cases, you can't really know if an environment variable will be defined or not, so just doing VARIABLE: $VARIABLE:blah:blah/blah isn't safe, because if VARIABLE isn't defined, there would be a colon at the beginning and that breaks things in some cases. Even if, by trial, you see that the variable is undefined, that could change in the future, so I think that it's a good thing for forward compatibility. |
Contributor
Author
|
@zyga Another user with this same problem: https://forum.snapcraft.io/t/using-expansion-syntax-in-apps-ld-library-path/47423 |
mr-cal
approved these changes
Aug 12, 2025
mr-cal
left a comment
mr-cal
left a comment
There was a problem hiding this comment.
This looks like a useful feature. Snapcraft shouldn't need any changes to support this because it doesn't validate or evaluate these environment variables.
We can update Snapcraft's docs here to recommend using this syntax, since it's a safer practice.
Contributor
There was a problem hiding this comment.
Pull request overview
Adds support for bash-style conditional parameter expansion in Environment.ExtendWithExpanded, enabling safer path-like concatenations in snapcraft.yaml-provided environment definitions.
Changes:
- Introduce conditional expansion handling for
${VAR:-word}and${VAR:+word}during environment expansion. - Route
ExtendWithExpandedvalue expansion through the new conditional-aware expansion helper. - Add unit tests covering the new conditional expansion behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
osutil/env.go |
Adds conditional expansion parsing/handling and updates ExtendWithExpanded to use it. |
osutil/env_test.go |
Adds tests validating conditional expansion results for various variables. |
Contributor
|
@sergio-costas can you look at the latest comments and check whether there's anything to address? And let's try to land it for 2.76. |
Contributor
Author
|
@bboozzoo On it! |
sergio-costas
force-pushed
the
allow-conditional-environment-variables
branch
from
April 6, 2026 11:34
9928edd to
2b07c4a
Compare
Member
|
Discussed with @sergio-costas, this is not very high priority. We will drop it from 2.76 release. |
sergio-costas
force-pushed
the
allow-conditional-environment-variables
branch
from
May 21, 2026 15:45
a314876 to
2561706
Compare
pedronis
force-pushed
the
allow-conditional-environment-variables
branch
from
May 29, 2026 12:08
2561706 to
0909f8b
Compare
Contributor
|
I did a bit of reviewing and pushed some tweaks myself |
bboozzoo
reviewed
Jun 3, 2026
pedronis
changed the title
Bash has support for some conditionals that allow to append text
to environment variables in a more clever way than just blindly
appending it. This is a must, for example, when appending paths
to PATH or LD_LIBRARY_PATH, because if the original variables
are empty, the new paths must be added "as-is", but if the
original variables have a value, a colon is required between
the old value and the new one.
This patch allows to append extra paths to the current defined
ones directly in the 'environment' section of the snapcraft.yaml
file.
The new supported syntaxes are:
${VARIABLE:-XXXXXX}
if VARIABLE is defined, this will return the value of that
variable; but if it is undefined, it will return the expansion
of XXXXX (thus, if there are variables there, they will be
recursively expanded).
${VARIABLE:+YYYYY}
if VARIABLE is undefined, it will return an empty string;
but if it is defined, it will return the expansion of YYYYY.
Example:
${PATH:+$PATH:}/usr/local/bin
If PATH is undefined or empty, it will return an empty string,
to with '/usr/local/bin' will be appended.
But if PATH contains, for example, /usr/bin:/bin, it will
return that plus a colon at the end, to which '/usr/local/bin'
will be appended, thus resulting in the expected value
/usr/bin:/bin:/usr/local/bin
Co-authored-by: Maciej Borzecki <maciek.borzecki@gmail.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
sergio-costas
force-pushed
the
allow-conditional-environment-variables
branch
from
June 8, 2026 08:18
8d6b9f7 to
c76b0ea
Compare
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
sergio-costas
force-pushed
the
allow-conditional-environment-variables
branch
from
June 8, 2026 08:21
c76b0ea to
b929b7d
Compare
Contributor
Author
|
Rebased to trigger a test |
github-actions
Bot
added
the
Auto rerun spread
Contributor
Author
|
Thanks! I'll add the documentation. |
Contributor
Author
|
@olivercalder I think that here is the best place to put it: https://documentation.ubuntu.com/snapcraft/latest/reference/snapcraft-yaml/#environment . I think that I should add a snapd version number... which will be the next release version? |
Member
Yes that looks like the right place. The change will be included in the snapd 2.77 release. |
Contributor
Author
|
Added canonical/snapcraft#6290 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bash has support for some conditionals that allow to append text
to environment variables in a more clever way than just blindly
appending it. This is a must, for example, when appending paths
to PATH or LD_LIBRARY_PATH, because if the original variables
are empty, the new paths must be added "as-is", but if the
original variables have a value, a colon is required between
the old value and the new one.
So, this patch allows to append extra paths to the current defined
ones directly in the 'environment' section of the snapcraft.yaml
file.
The new supported syntaxes are:
if VARIABLE is defined, this will return the value of that
variable; but if it is undefined, it will return the expansion
of XXXXX (thus, if there are variables there, they will be
recursively expanded).
if VARIABLE is undefined, it will return an empty string;
but if it is defined, it will return the expansion of YYYYY.
Examples:
If PATH is undefined or empty, it will return an empty string,
to with '/usr/local/bin' will be appended.
But if PATH contains, for example, /usr/bin:/bin, it will
return that plus a colon at the end, to which '/usr/local/bin'
will be appended, thus resulting in the expected value
If PATH is undefined or empty, it will return an empty string,
which will be appended after '/usr/local/bin', thus resulting in only that path.
But if PATH contains, for example, /usr/bin:/bin, it will
return a colon plus that at the end, which will be appended after '/usr/local/bin', thus resulting in the expected value
Thanks for helping us make a better snapd!
Have you signed the license agreement and read the contribution guide?