Skip to content

Conversation

@am11
Copy link
Member

@am11 am11 commented Jun 16, 2024

Fix #1088

@am11 am11 requested a review from a team as a code owner June 16, 2024 07:10
@ghost ghost added the untriaged label Jun 16, 2024
@ghost
Copy link

ghost commented Jun 16, 2024

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

1 similar comment
@ghost
Copy link

ghost commented Jun 16, 2024

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

@am11 am11 force-pushed the feature/move-images/risv64 branch 6 times, most recently from 5dbe9e2 to bb00447 Compare June 16, 2024 15:06
@am11 am11 force-pushed the feature/move-images/risv64 branch from bb00447 to c5b80ef Compare June 16, 2024 16:44
@am11
Copy link
Member Author

am11 commented Jun 16, 2024

cc @akoeplinger @sbomer

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.

LGTM besides one comment


# Install the latest debootstrap
RUN tdnf remove -y debootstrap && \
git clone --depth 1 "https://git.launchpad.net/ubuntu/+source/debootstrap" /debootstrap && \
Copy link
Member

Choose a reason for hiding this comment

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

from what I can see noble was added in debootstrap 1.0.134 so I'd prefer getting the .tar.gz from https://ftp.debian.org/debian/pool/main/d/debootstrap/ instead of git

Copy link
Member

Choose a reason for hiding this comment

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

also would be good to file an issue tracking this so we can remove it once azurelinux is updated

Copy link
Member Author

Choose a reason for hiding this comment

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

It is a shell script, not an executable. We will need to setup a fakechroot etc. to make it work from the tarball, see https://src.fedoraproject.org/rpms/debootstrap/blob/rawhide/f/debootstrap.spec. BTW, these steps are taken from the readme of lanchpad official package, to run without installing. Lets keep this for now. I will remove it once newer version of azlinux is published in a few months.

Copy link
Member

Choose a reason for hiding this comment

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

I looked into this a bit more and it seems the only relevant change is that scripts/noble is a symlink to scripts/gutsy. I think we can just create that symlink?

Copy link
Member Author

Choose a reason for hiding this comment

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

This step took 8s (out of 145mins)

2024-06-16T18:06:48.9108010Z #7 [builder 2/4] RUN tdnf remove -y debootstrap &&     git clone --depth 1 "https://git.launchpad.net/ubuntu/+source/debootstrap" /debootstrap &&     chmod a+x /debootstrap/debootstrap &&     ln -s /debootstrap/debootstrap -t /usr/local/bin
2024-06-16T18:06:49.0377831Z #7 0.284 Loaded plugin: tdnfrepogpgcheck
2024-06-16T18:06:50.5655548Z #7 1.812 
2024-06-16T18:06:50.5655730Z #7 1.812 Removing:
2024-06-16T18:06:50.5656173Z #7 1.812 debootstrap      noarch       1.0.128+nmu2-1.azl3 @System      193.98k     0.00b
2024-06-16T18:06:50.5658780Z #7 1.812 
2024-06-16T18:06:50.5659290Z #7 1.812 Total installed size: 193.98k
2024-06-16T18:06:50.5659535Z #7 1.812 Total download size:   0.00b
2024-06-16T18:06:50.6800990Z #7 1.927 Testing transaction
2024-06-16T18:06:50.8734176Z #7 1.930 Running transaction
2024-06-16T18:06:50.8734709Z #7 1.974 Removing: debootstrap-1.0.128+nmu2-1.azl3.noarch
2024-06-16T18:06:50.8735050Z #7 2.120 Cloning into '/debootstrap'...
2024-06-16T18:06:56.8048205Z #7 DONE 8.1s

riscv64 is a community-supported platform, I think we can live with it for a while. Unless you are feeling too strongly about it, I'd say lets keep it for now and we can delete it once tdnf picks up the latest version in a few weeks or so. (we have been using and trusting launchpad aka Canonical packages forever, so there is no security risk)

LMK 😅

Copy link
Member

Choose a reason for hiding this comment

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

@am11 it's not about the time, I'm more worried about cloning directly from git HEAD. We don't really know when Azure Linux will bump to new debootstrap and if a simple RUN ln -s gutsy /usr/share/debootstrap/scripts/noble works too I prefer that :)

In fact I tried that and it seems to work for me locally.

Copy link
Member Author

Choose a reason for hiding this comment

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

Lets merge this to unblock others and then follow-up on it. I don't wana run another 4 hours CI job for it. That git-clone step hasn't changed in years and hasn't broken anyone. It can sustain a few days. Fair?

Copy link
Member

Choose a reason for hiding this comment

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

I agree, sounds good.

Copy link
Member Author

Choose a reason for hiding this comment

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

@akoeplinger akoeplinger merged commit fe7c8a1 into dotnet:main Jun 17, 2024
@am11 am11 deleted the feature/move-images/risv64 branch June 17, 2024 18:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Building rootfs fails for ubuntu-22.04-cross-riscv64

2 participants