Skip to content
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.

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

Inline apt source generation in rootfs script #14849

Merged
merged 8 commits into from
Jun 21, 2024

Conversation

am11
Copy link
Member

@am11 am11 commented Jun 13, 2024

All our supported distros support deb822 format (circa 2015 onwards). Inline the contents of those redundant files and simplify code.

cc @akoeplinger @sbomer needs a few more tests on raspberry pi etc. help would be appreciated. :)

@akoeplinger
Copy link
Member

These scripts are mainly for our own usage so if all of the distros we support can handle it I'm fine :)

@am11
Copy link
Member Author

am11 commented Jun 17, 2024

Yup, these days we can use this script to also setup a cross-builder image for nativeaot dotnet/runtime#97729 (comment). With this change,

RUN mkdir /dev/arm; \
  curl -sSL https://raw.githubusercontent.com/dotnet/arcade/main/eng/common/cross/arm/sources.list.jammy -o /dev/arm/sources.list.jammy; \
  curl -sSL https://raw.githubusercontent.com/dotnet/arcade/main/eng/common/cross/build-rootfs.sh |\
    bash /dev/stdin arm jammy llvm15 lldb15

will shrink down to:

RUN mkdir /dev/arm; \
  curl -sSL https://raw.githubusercontent.com/dotnet/arcade/main/eng/common/cross/build-rootfs.sh |\
    bash /dev/stdin arm jammy llvm15 lldb15

@am11
Copy link
Member Author

am11 commented Jun 20, 2024

Testing in CI dotnet/dotnet-buildtools-prereqs-docker#1102.

@am11
Copy link
Member Author

am11 commented Jun 21, 2024

dotnet/dotnet-buildtools-prereqs-docker#1102 is green, so it's working as before. Needed to make some adjustments here to make it work.

@akoeplinger, @janvorli, @sbomer, PTAL.

@am11 am11 marked this pull request as ready for review June 21, 2024 09:09
@am11
Copy link
Member Author

am11 commented Jun 21, 2024

@gbalykov, FYI. We can also inline Tizen script and using heredoc syntax, the patches could be inlined as well (to make this script completely freestanding).

@@ -292,10 +292,13 @@ while :; do
if [[ "$__CodeName" != "jessie" ]]; then
__CodeName=noble
fi
__UseDeb822Format=1
if [[ -n "$__LLDB_Package" ]]; then
__LLDB_Package="liblldb-18-dev"
Copy link
Member

Choose a reason for hiding this comment

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

I don't really understand why and when we set __LLDB_Package for the various distros, would you mind explaining?

Copy link
Member Author

@am11 am11 Jun 21, 2024

Choose a reason for hiding this comment

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

When we invoke build-rootfs.sh <arch> <codename> lldbX, it uses liblldb-X-dev. When we don't provide lldbX, it uses the default 3.9. llvm/lldb v3.9 is not available in 24.04 (noble), so I'm changing its default. We have used noble branch only for riscv64 until now, where lldb is not yet available in debian/ubuntu packages.

The current approach of setting defaults, parsing args, overriding defaults based on arguments, then figuring out the "order" of args (what makes sense for overriding and when) is very twisted. I think we should refactor this script and define presets: combination of mandatory arguments. And remove all default/overriding handing. It will be a bit verbose, but it will make things readable for someone trying to work with it.

Copy link
Member

Choose a reason for hiding this comment

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

Ok thanks, I agree we should refactor the defaults.

Copy link
Member

@janvorli janvorli left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!

@akoeplinger akoeplinger merged commit 3aba80f into dotnet:main Jun 21, 2024
11 checks passed
@am11 am11 deleted the feature/build-rootfs-freestanding branch June 21, 2024 16:00
@gbalykov
Copy link
Member

gbalykov commented Jun 24, 2024

FYI. We can also inline Tizen script and using heredoc syntax, the patches could be inlined as well (to make this script completely freestanding).

@am11 Thanks for sharing information! We'll probably work on this a bit later, busy with other tasks right now, cc @dotnet/samsung

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.

4 participants