Skip to content

Add curl support in build-rootfs.sh#14744

Merged
akoeplinger merged 3 commits intodotnet:mainfrom
am11:patch-11
May 7, 2024
Merged

Add curl support in build-rootfs.sh#14744
akoeplinger merged 3 commits intodotnet:mainfrom
am11:patch-11

Conversation

@am11
Copy link
Member

@am11 am11 commented Apr 26, 2024

There are systems which come preinstalled with wget, curl, both or none. To make things smoother, this PR adds curl support side-by-side wget. We have this flexibility in other scripts as well.

@am11
Copy link
Member Author

am11 commented Apr 29, 2024

@akoeplinger, PTAL.

@akoeplinger
Copy link
Member

I don't have a strong opinion but I wonder if it'd be easier to read and more maintainable to just do a if haveWget check and use wget or curl directly whenever we need it.

@am11
Copy link
Member Author

am11 commented May 4, 2024

Switched to inline view. I agree it's more readable (easy to copy-paste and test in isolation).

@am11 am11 force-pushed the patch-11 branch 2 times, most recently from 7aea4d9 to 8d1b8d6 Compare May 4, 2024 13:44
Copy link
Member

@akoeplinger akoeplinger left a comment

Choose a reason for hiding this comment

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

One comment, LGTM otherwise

elif command -v curl &> /dev/null; then
__hasWget=0
else
>&2 echo "ERROR: either wget or curl is required by this script."
Copy link
Member

Choose a reason for hiding this comment

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

the ubuntu/debian/tizen cases don't need wget/curl today so I'd prefer not erroring to keep the same behavior

Copy link
Member Author

Choose a reason for hiding this comment

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

Extracted into ensureDownloadTool() function.

Copy link
Member

@akoeplinger akoeplinger left a comment

Choose a reason for hiding this comment

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

Thank you!

@akoeplinger akoeplinger merged commit f663d56 into dotnet:main May 7, 2024
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