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
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
| @@ -34,7 +36,135 @@ makedepends=( | |||
| '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.
| '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.
| @@ -24,7 +23,10 @@ depends=( | |||
| '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.
| python -m installer --destdir="$pkgdir" dist/*.whl | ||
| 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?
| @@ -57,21 +187,22 @@ pkgver() { | |||
|
|
|||
| prepare() { | |||
| cd $pkgname-$pkgver | |||
|
|
|||
| # use real directories for examples and profiles, as symlinks do not work | |||
| rm -fv $pkgname/{examples,profiles} | |||
There was a problem hiding this comment.
Have those symbolic links been removed/changed?
There was a problem hiding this comment.
Yepp, they've moved and been re-worked. Official PKGBUILD (refered to above) does not have it and works.
| [nvchecker] | ||
| source = "git" | ||
| git = "https://github.com/archlinux/archinstall" | ||
| prefix = "v" |
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.
| pkgrel=1 | ||
| pkgdesc="Just another guided/automated Arch Linux installer with a twist" | ||
| 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.
| 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. |
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.