Skip to content

Don't expand variables when collecting directives#4871

Open
lukaszachy wants to merge 2 commits into
mainfrom
heredoc-4867
Open

Don't expand variables when collecting directives#4871
lukaszachy wants to merge 2 commits into
mainfrom
heredoc-4867

Conversation

@lukaszachy
Copy link
Copy Markdown
Contributor

Fix: #4867

Pull Request Checklist

  • implement the feature

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request updates the bootc package manager to quote the EOF delimiter in a heredoc, preventing unintended shell expansions. A review comment suggests removing trailing whitespace before newline characters in the ShellScript string to ensure the Containerfile is generated with exact formatting.

Comment on lines +262 to +263
# Quoted delimiter disables any shell expansions
ShellScript(f'cat <<"EOF" > {containerfile_path!s} \n{containerfile} \nEOF')
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The spaces before \n in the ShellScript string result in trailing whitespace in the generated Containerfile. Remove them to ensure the directives are written exactly as intended.

Suggested change
# Quoted delimiter disables any shell expansions
ShellScript(f'cat <<"EOF" > {containerfile_path!s} \n{containerfile} \nEOF')
# Quoted delimiter disables any shell expansions
ShellScript(f'cat <<"EOF" > {containerfile_path!s}\n{containerfile}\nEOF')
References
  1. Paths that are internally generated and controlled by the application do not need to be quoted in shell commands, as they are not considered user-provided input.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think it is/will be addresses in #4875

@happz happz added ci | full test Pull request is ready for the full test execution area | package managers Changes related to implementations of package managers labels May 7, 2026
@happz happz added this to planning May 7, 2026
@happz happz moved this to review in planning May 7, 2026
The 'finish' doesn't run, but prepare can be tested already
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area | package managers Changes related to implementations of package managers ci | full test Pull request is ready for the full test execution

Projects

Status: review

Development

Successfully merging this pull request may close these issues.

Unwated variable expansion when collecting directives for image-mode

3 participants