Skip to content

Use more descriptive long-form options for curl, tar, and checksum validation #6496

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 13 commits into from
Jun 20, 2025
Merged

Use more descriptive long-form options for curl, tar, and checksum validation #6496

merged 13 commits into from
Jun 20, 2025

Conversation

lbussell
Copy link
Contributor

@lbussell lbussell commented Jun 19, 2025 •
edited
Loading

Reading through our Dockerfiles can be a little confusing if you're not familiar with specific command line option abbreviations for curl, tar, etc.

This PR attempts to make them easier to read by using the long-form options:

  • curl -fSLO -> curl --fail --silent --location --output
  • tar -oxzf -> tar --gzip --extract --no-same-owner --file
  • sha512sum -c ... -> sha512sum --check ...

@lbussell lbussell requested review from benjaminpetit, a team, joperezr and wiktork as code owners June 19, 2025 17:26
mthalman
mthalman approved these changes Jun 19, 2025
View reviewed changes
Copy link
Member

@mthalman mthalman left a comment

Choose a reason for hiding this comment

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

This is limited to the three commands you described. Are there plans to expand this everywhere? At a glance, I can see ln and mkdir as candidates. I guess what's the overall plan here?

@richlander
Copy link
Member

Here's a HN post that describes much the same thing: https://news.ycombinator.com/item?id=43440184. It's useful context for folks (including us, later) who want to understand the motivation.

@lbussell
Copy link
Contributor Author

Yes, I acknowledge that I left out mkdir and ln (and probably others). I can add them. I will also update CONTRIBUTING.md accordingly.

mthalman
mthalman reviewed Jun 19, 2025
View reviewed changes
Copy link
Member

@mthalman mthalman left a comment

Choose a reason for hiding this comment

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

Other commands I see that can use long form are rm -rf and useradd -l.

lbussell added 2 commits June 19, 2025 15:12
@lbussell
Update more commands
ecd232e 
- tdnf install
- useradd
- apt-get install
- rm
@lbussell
Use sha512sum -c instead of --check for Alpine
8984acc
mthalman
mthalman reviewed Jun 20, 2025
View reviewed changes
@lbussell
Copy link
Contributor Author

lbussell commented Jun 20, 2025 •
edited
Loading

OK, here's what I did:

  • CONTRIBUTING.md now says the cli option choice is a preference, not a directive
  • Reverted rm -rf because it is obvious
  • Reverted ln -s and sha512sum -c because the alpine/busybox versions of those only support the shorthand arguments, and it keeps the templates simple and the Dockerfiles consistent
  • Reverted package manager assume-yes changes per your comment.

mthalman
mthalman approved these changes Jun 20, 2025
View reviewed changes
Copy link
Member

@mthalman mthalman left a comment

Choose a reason for hiding this comment

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

This strikes a good balance of short and long form, I think.

@lbussell
Copy link
Contributor Author

(Markdown link check failure is a false positive)

@lbussell lbussell merged commit 413bf98 into dotnet:nightly Jun 20, 2025
114 of 115 checks passed
@lbussell lbussell deleted the curl-args branch June 23, 2025 16:38
lbussell added a commit to lbussell/dotnet-docker that referenced this pull request Jul 7, 2025
@lbussell
Use more descriptive long-form options for curl, tar, and checksum va…
b3c1717 
…lidation (dotnet#6496)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mthalman mthalman mthalman approved these changes

@benjaminpetit benjaminpetit Awaiting requested review from benjaminpetit benjaminpetit is a code owner

@joperezr joperezr Awaiting requested review from joperezr joperezr is a code owner

@wiktork wiktork Awaiting requested review from wiktork wiktork is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@lbussell @richlander @mthalman