Adding nvchecker conf, listing most (or all) optional package dependencies and fixed license identifier #2406
Conversation
…etimes have breaking changes - this is a small step in trying to help us track these breaking changes
…can hit HTTP limits
dvzrv
left a comment
dvzrv
left a comment
There was a problem hiding this comment.
Generally I am wondering why these files live in here at all and why this has not been a merge request towards the package repository instead:
https://gitlab.archlinux.org/archlinux/packaging/packages/archinstall/-/merge_requests
| 'python-wheel' | ||
| ) | ||
| optdepends=( | ||
| 'linux: Used when linux kernel is selected' |
There was a problem hiding this comment.
The newly added optdepends are not really needed for running archinstall though.
They will be installed on-the-fly into a newly setup system (which is not the same system as the one running archinstall).
Therefore they should not be added. Only optional dependencies required for additional features of archinstall should be added here.
There was a problem hiding this comment.
For anyone reading -> Ongoing discussion internally about how to solve the core need of why I've attempted to add them here.
There was a problem hiding this comment.
I need to re-visit this, and get feedback from other packages.
Because it happens from time to time, and today it struck again: https://gitlab.archlinux.org/archlinux/packaging/packages/xf86-video-vmware/-/issues/3#note_277232
There's no other way for me to link "indirect" package ties between a package like xf86-video-vmware to archinstall is there?
There was a problem hiding this comment.
Even if you created a metapackage with depends on all the things archinstall can try to install, I'm not sure it would have helped. Anyone removing a package from our repos must still manually double-check for depends, just as for optdepends.
I think what we want is for dbscripts to disallow removing packages that are still referenced by any {,make,opt,check}depends.
There was a problem hiding this comment.
In such a case, this list still makes sense? But in a meta package and then enhance dbscripts? This sounds like a very viable option tbh!
There was a problem hiding this comment.
If dbscripts can be enhanced, either way should work.
There was a problem hiding this comment.
That said, having metapackages like archinstall-meta-desktop-budgie that define what gets installed when the budgie desktop is selected (instead of archinstall's code) has some appeal, IMO.
| 'mkinitcpio' | ||
| 'util-linux' | ||
| 'base' | ||
| 'base-devel' |
There was a problem hiding this comment.
Why is base-devel added?
If archinstall now depends on particular build tools to do its job, those need to be listed here instead.
| 'python-pyparted' | ||
| 'python-simple-term-menu' | ||
| 'systemd' | ||
| 'mkinitcpio' |
There was a problem hiding this comment.
Does archinstall use mkinitcpio on the system running archinstall or in the new system?
There was a problem hiding this comment.
It also triggers mkinitcpio inside the system, to make sure modifications to HOOKS etc are updated before rebooting.
| install -vDm 644 docs/_build/man/archinstall.1 -t "$pkgdir/usr/share/man/man1/" | ||
| } | ||
|
|
||
| check() { |
There was a problem hiding this comment.
If check() does nothing, it should not be here.
Once it calls something, it should be ordered before package() as that is when it is called.
There was a problem hiding this comment.
To not forget that we want tests, is it fine if I comment it out instead of removing it?
| [nvchecker] | ||
| source = "git" | ||
| git = "https://github.com/archlinux/archinstall" | ||
| prefix = "v" No newline at end of file |
There was a problem hiding this comment.
It seems your editor does not like newlines at the end of the file.
Might be worth changing, since PKGBUILDs usually use that by default as well and it will turn into a non-needed change every time otherwise.
| arch=(any) | ||
| url="https://github.com/archlinux/archinstall" | ||
| license=(GPL3) | ||
| license=(GPL-3.0-or-later) |
There was a problem hiding this comment.
According to the project information it is GPL-3.0-only though?
There was a problem hiding this comment.
hmm, let me double check. if that's the case I'll change.
| license=(GPL-3.0-or-later) | ||
| depends=( | ||
| 'arch-install-scripts' | ||
| 'btrfs-progs' |
There was a problem hiding this comment.
Does archinstall no longer support btrfs or is support for the filesystem achieved in another way?
There was a problem hiding this comment.
humm, crap you're right. It needs it in the ISO to also be able to do mkfs.btrfs. Good catch! I was thinking it just needed it to be installed inside the system.
It's because: To get the latest code for testing, we have archinstall-git option too but this was added way back before that existed as an option. |
|
We'll create a meta package containing all the dependencies, to not bloat this one. |
This PR mainly aims to conform with Arch's best practices surrounding PKGBUILD's.
check()hook for future test suit - https://wiki.archlinux.org/title/Creating_packages#check()optdependsas they are optional dependencies and used when the user selects a relevant profile. This should be a good step for future fail-safe's regarding package name changes etc. As we can cross-reference from other packages back to archinstall when we change other packages.