Skip to content

Conversation

@x0root
Copy link

@x0root x0root commented Jan 2, 2025

Ensure that the exec command properly handles arguments when using exec "$python" $extra_arg "$xpy" "$@". The problem with this approach is that $extra_arg is unquoted, which could cause issues if it contains spaces or other special characters, especially when passing the arguments to python. To fix this, the exec command should be properly quoted to prevent any such issues.

Ensure that the exec command properly handles arguments when using exec "$python" $extra_arg "$xpy" "$@". The problem with this approach is that $extra_arg is unquoted, which could cause issues if it contains spaces or other special characters, especially when passing the arguments to python. To fix this, the exec command should be properly quoted to prevent any such issues.
@rustbot
Copy link
Collaborator

rustbot commented Jan 2, 2025

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Mark-Simulacrum (or someone else) some time within the next two weeks.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

  • @rustbot author: the review is finished, PR author should check the comments and take action accordingly
  • @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Jan 2, 2025
@jieyouxu
Copy link
Member

jieyouxu commented Jan 2, 2025

No, this is not needed because that is a local var which is only ever assigned "" or "-3", it does not accept arbitrary (user) input.

@jieyouxu jieyouxu closed this Jan 2, 2025
@rust-log-analyzer
Copy link
Collaborator

The job mingw-check failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
configure: writing `config.toml` in current directory
configure: 
configure: run `python /checkout/x.py --help`
##[endgroup]
/usr/bin/python3: can't find '__main__' module in '/checkout/obj/'
  network time: Thu, 02 Jan 2025 07:23:59 GMT
##[error]Process completed with exit code 1.
Post job cleanup.

@RalfJung
Copy link
Member

RalfJung commented Jan 2, 2025

No, this is not needed because that is a local var which is only ever assigned "" or "-3", it does not accept arbitrary (user) input.

In fact the change proposed by the PR is just wrong -- if the variable is empty, we want no argument to be passed, but wrapping it in quotes means we would pass an empty argument instead!

@saethlin saethlin mentioned this pull request Jan 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants