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

T6333 non-free-firmware to trixie #614

Merged
merged 1 commit into from
May 16, 2024
Merged

Conversation

aidan-gibson
Copy link
Contributor

Change Summary

Fixes this warning that would otherwise occur:

N: Repository 'Debian bookworm' changed its 'non-free component' value from 'non-free' to 'non-free non-free-firmware'
N: More information about this can be found online in the Release notes at: https://www.debian.org/releases/bookworm/amd64/release-notes/ch-information.html#non-free-split

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes)
  • Migration from an old Vyatta component to vyos-1x, please link to related PR inside obsoleted component
  • Other (please describe):

Related Task(s)

https://vyos.dev/T6333

Component(s) name

Proposed changes

How to test

Checklist:

  • I have read the CONTRIBUTING document
  • I have linked this PR to one or more Phabricator Task(s)
  • My commit headlines contain a valid Task id
  • My change requires a change to the documentation
  • I have updated the documentation accordingly

@sever-sever sever-sever requested review from a team, dmbaturin, sarthurdev, zdc, jestabro, sever-sever and c-po and removed request for a team May 13, 2024 14:58
@Apachez-
Copy link
Contributor

Isnt this already handled by this:

https://github.com/vyos/vyos-build/blob/current/data/defaults.toml

Also the non-free-firmware shouldnt that exist for trixie-updates aswell?

@aidan-gibson
Copy link
Contributor Author

aidan-gibson commented May 13, 2024

current/data/defaults.toml just accounts for bookworm, not trixie.

I believe you're correct about trixie-updates, good catch! I'll try rebuilding with that just to make sure there are no surprises. I feel like there shouldn't be but I'm an absolute novice with apt, my main OS is arch, so I don't trust my judgment with apt yet.

@aidan-gibson
Copy link
Contributor Author

In regards to current/data/defaults.toml actually, I wonder if the error will clear if debian_distribution is set to trixie instead. I don't completely understand the build process of VyOS, but I suspect data/live-build-config/archives/trixie.list.chroot is logically separate from what's going on with current/data/defaults.toml (so this minor edit is needed regardless). If someone more experienced with VyOS reads this, feel free to jump in.

@Apachez-
Copy link
Contributor

I feel that this PR should be denied because changing to "trixie" from "bookworm" is done at the wrong place.

It should be handled either by cmdline or by files at https://github.com/vyos/vyos-build/blob/current/data/ based on https://github.com/vyos/vyos-build/blob/current/data/defaults.toml as a start.

That is alter:

debian_distribution = "bookworm"

into:

debian_distribution = "trixie"

Also note that current 1.5-rolling is based on bookworm so suddently switching to trixie (which is the upcoming Debian version after bookworm) might break stuff.

If you still want to have a go I would just manually alter the content of defaults.toml in my build environment instead of adding new files to the VyOS repo.

@aidan-gibson
Copy link
Contributor Author

@Apachez- I think you're a little mixed up with VyOS builds (as am I honestly)--not all packages come from bookworm. Different packages are pinned to different Debian versions. It is very noticable when building, you see something like:
image

@aidan-gibson
Copy link
Contributor Author

Take a look at the files in this dir, for example: https://github.com/vyos/vyos-build/tree/current/data/live-build-config/archives

See what I'm saying?

@jestabro
Copy link
Contributor

jestabro commented May 13, 2024

@aidan-gibson you are correct that it applies only to those packages pinned in:
https://github.com/vyos/vyos-build/blob/current/data/live-build-config/archives/trixie.pref.chroot
hence needed independent of defaults.toml.
Cf. https://vyos.dev/T5867; introduced in:
197feda

@aidan-gibson
Copy link
Contributor Author

@jestabro Thanks for the info! Do you think line 2, deb http://deb.debian.org/debian/ trixie-updates main non-free should also have non-free-firmware appended? I'm seeing the same output on builds with or without; it doesn't seem to make a difference for now but maybe could cause issues down the road, one way or another.

@Apachez- The purpose of this PR is to eliminate a warning we get on builds, as stated initially. If you have alternative code that eliminates the warning in a different fashion, I'd love to learn from you, I still very much consider myself a VyOS novice. The PR I'd really like to implement is intel-microcode and amd64-microcode being included in VyOS (https://vyos.dev/T6322), but I'm unsure the best way to do that--I found this minor error/patch while trying to figure out the best way to do that.

Best regards to you both! I'm having a fantastic time with VyOS thus far.

@Apachez-
Copy link
Contributor

If you just want to add 2 additional packages to your custom build the easiest way to do this is probably in your local build environment modify https://github.com/vyos/vyos-build/blob/current/data/architectures/amd64.toml to include the extra packages.

You can also add the custom packages through cmdline when compiling the ISO such as --custom-package:

https://docs.vyos.io/en/latest/contributing/build-vyos.html#customize

@aidan-gibson
Copy link
Contributor Author

Thanks I'll try that build now! Yep, --custom-package worked without a hitch, I'm just flabberghasted as to why it didn't work when adding them to where the majority of the packages go. The dearth of packages spooked me from submitting a PR trying out that dir, current/data/architectures/amd64.toml, but screw it, if it works I'll PR it and worst case the person rejecting the PR will know the proper way to do this.

@c-po c-po merged commit a33e9cf into vyos:current May 16, 2024
2 of 3 checks passed
@c-po
Copy link
Member

c-po commented May 16, 2024

@Mergifyio backport sagitta

Copy link

mergify bot commented May 16, 2024

backport sagitta

✅ Backports have been created

c-po added a commit that referenced this pull request May 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

4 participants