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

Replace debootstrap with mmdebstrap #181

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Replace debootstrap with mmdebstrap #181

wants to merge 7 commits into from

Conversation

upils
Copy link
Collaborator

@upils upils commented Jan 30, 2024

To speed up the build time and for various future improvements we are replacing debootstrap with mmdebstrap.

Bonus: the test suite is now ~10min shorter.

Fixes: FR-6576

Copy link

codecov bot commented Jan 30, 2024

Codecov Report

Attention: Patch coverage is 89.18919% with 4 lines in your changes are missing coverage. Please review.

Project coverage is 94.19%. Comparing base (2be85a7) to head (2722bec).

Current head 2722bec differs from pull request most recent head 89fb232

Please upload reports for the commit 89fb232 to get more accurate results.

Files Patch % Lines
internal/statemachine/classic_states.go 90.47% 1 Missing and 1 partial ⚠️
internal/testhelper/testhelper.go 60.00% 2 Missing ⚠️
Additional details and impacted files
@@              Coverage Diff               @@
##           use-core24     #181      +/-   ##
==============================================
- Coverage       94.27%   94.19%   -0.08%     
==============================================
  Files              16       16              
  Lines            3284     3310      +26     
==============================================
+ Hits             3096     3118      +22     
- Misses            120      123       +3     
- Partials           68       69       +1     
Flag Coverage Δ
unittests 94.19% <89.18%> (-0.08%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mwhudson
Copy link
Contributor

I support this in general but are we sure that mmdebstrap behaviour is the same or sufficiently close to the same as debootstrap?

@upils
Copy link
Collaborator Author

upils commented Jan 30, 2024

I support this in general but are we sure that mmdebstrap behaviour is the same or sufficiently close to the same as debootstrap?

I am investigating. There is a card in Jira with more context and this PR is to determine how hard it would be to replace debootstrap with mmdebstrap. From what I can read from the wiki, the README and the source code, they try to stay close to the result debootstrap produce, but try to do it more efficiently and with more options.

So far I think I have found a decent set of arguments to reproduce the chroot we build with debootstrap but I need to compare the package lists and probably go further to compare the resulting chroots. If you know any efficient method to do so, I am interested :).

@mwhudson
Copy link
Contributor

I am investigating. There is a card in Jira with more context and this PR is to determine how hard it would be to replace debootstrap with mmdebstrap. From what I can read from the wiki, the README and the source code, they try to stay close to the result deboostrap produce, but try to do it more efficiently and with more options.

Great.

So far I think I have found a decent set of arguments to reproduce the chroot we build with debootstrap but I need to compare the package lists and probably go further to compare the resulting chroots. If you know any efficient method to do so, I am interested :).

I don't, I'm afraid. I only got as far as "run debootstrap mantic, run mmdebstrap mantic, notice the results are not identical" ...

@upils
Copy link
Collaborator Author

upils commented Feb 6, 2024

When trying to build the current ubuntu-server-amd64 image, u-i with mmdebstrap is failing:

[...]
Setting up nano (7.2-2) ...
update-alternatives: error: alternative path /bin/nano doesn't exist
dpkg: error processing package nano (--configure):
 installed nano package post-installation script subprocess returned error exit status 2
[...]

Currently u-i as a snap is base on core22, so the installed version of mmdebstrap is 0.8.4. The changelog for more recent version mention several fixes to implement the same behavior as debootstrap regarding usrmerge.

I see several possible solutions:

  • wait for core24 (should bring mmdebstrap 1.4.3, or at least 1.4.0)
  • create a snap for mmdebstrap with a recent version (will probably require some non trivial work and a maintenance cost)
  • SRU mmdebstrap 1.4.3 in jammy (very unlikely I think)

I think the first solution is the best one because we are not in a hurry to replace debootstrap for now.

@upils upils self-assigned this Feb 6, 2024
@upils upils changed the title FR-6313 - Replace debootstrap with mmdebstrap FR-6576 - Replace debootstrap with mmdebstrap Feb 9, 2024
@bdrung
Copy link

bdrung commented Feb 15, 2024

Note: You can use diffoscope to compare the output of debootstrap and mmdebstrap to actually see the difference.

@upils upils changed the title FR-6576 - Replace debootstrap with mmdebstrap Replace debootstrap with mmdebstrap Mar 11, 2024
@upils
Copy link
Collaborator Author

upils commented Mar 11, 2024

Apart from a specific error with qemu for one build, this is now working (no need for core24). I should now compare outputs using diffoscope.

@upils upils force-pushed the use-mmdebstrap branch 2 times, most recently from 7a90854 to 2722bec Compare May 2, 2024 14:08
@upils upils requested a review from sil2100 May 2, 2024 14:46
@upils upils marked this pull request as ready for review May 2, 2024 14:46
@upils upils changed the base branch from main to use-core24 May 27, 2024 09:02
@upils
Copy link
Collaborator Author

upils commented May 27, 2024

I rebased on #223 because in the version of mmdebstrap in jammy (0.8.4), some paths to look for some tools (arch-test, update-binfmt, etc.) are absolutes and so packages in the snap cannot be found by mmdebstrap.

So this is in standby until #223 is merged.

@upils upils force-pushed the use-core24 branch 2 times, most recently from 62cf97b to b018cf9 Compare July 5, 2024 11:47
Base automatically changed from use-core24 to main July 5, 2024 12:56
@j5awry
Copy link

j5awry commented Jul 15, 2024

QQ -- with mmdebstrap in universe, is there a plan to move it to main, for full support? regardless of packaging, it seems odd to have something meant to be a fundamental piece of software for Ubuntu relying on a universe package. May cause issues with consumers.

@upils
Copy link
Collaborator Author

upils commented Jul 16, 2024

QQ -- with mmdebstrap in universe, is there a plan to move it to main, for full support? regardless of packaging, it seems odd to have something meant to be a fundamental piece of software for Ubuntu relying on a universe package. May cause issues with consumers.

Nice catch. Since in the end we only package ubuntu-image as a snap, consumers are supposed to have "everything" and not really worry of the details on this. But from our maintainer's point of view I agree it could also be a problem. I will discuss it and see if this is worth a MIR process.

@sil2100 What do you think?

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