freebsd / poudriere

Port/Package build and test system
https://github.com/freebsd/poudriere/wiki
BSD 2-Clause "Simplified" License
379 stars 161 forks source link

Options broken in poudriere-devel #1090

Closed mattxtaz closed 6 months ago

mattxtaz commented 7 months ago

Prerequisites

Describe the bug

When I run poudriere options it displays an error. This worked in the previous version and the issue started happening since the last port upgrade.

How to reproduce

Steps to reproduce the behavior:

% poudriere options -n -f /usr/local/etc/poudriere.d/optlist
[00:00:00] Working on options directory: /usr/local/etc/poudriere.d/options
[00:00:00] Using ports from: /usr/ports
[00:00:00] Appending to make.conf: /usr/local/etc/poudriere.d/make.conf
[00:00:00] Error: _jget: 3 arguments expected: var_return ptname property

Expected behavior

poudriere should loop through all of the ports in the options file to see if any of the options need to present a dialog to update them in the case of any options being added or removed.

Screenshots

## Environment - Host OS [e.g. 12.2 amd64]: 13.2 amd64 - Jail OS [e.g. 12.0 powerpc]: 13.2 amd64 - Browser: [e.g. chrome, safari]: Firefox - Poudriere Version [e.g. 3.3.1 or git hash or port version]: poudriere-devel-3.4.99.20231113 - Ports branch and revision [e.g. 2020Q3 r550754]: latest (259ddfbbd104e024361591fc5bec5a252c6eb5cb) ## Additional context
mattxtaz commented 7 months ago

Just adding another comment in case it's important. /usr/local/etc/poudriere.d/options is a symlink to /var/db/ports so that I can use make config in the /usr/ports directories.

ttyva commented 7 months ago

+1 options.sh is trying to start a jail with an empty name. Manually specifying the target arch doesn't help. This breaks the use case where options are set without a specific jail (by ports tree+set for us).

fernape commented 7 months ago

I've been bitten by this today.

It is broken regardless of the jail name. Without the name of the jail, it shows that weird error. With the name, it reports the port does not exist (but it does, and poudriere can built it).

pkg info poudriere-devel
poudriere-devel-3.4.99.20231113
$ sudo poudriere options -p default databases/sql-workbench
Password:
[00:00:00] Working on options directory: /usr/local/etc/poudriere.d/default-options
[00:00:00] Using ports from: /usr/local/poudriere/ports/default
[00:00:00] Appending to make.conf: /usr/local/etc/poudriere.d/make.conf
[00:00:00] Error: _jget: 3 arguments expected: var_return ptname property
$ sudo poudriere options -p default -j 13_2amd64 databases/sql-workbench
[00:00:00] Working on options directory: /usr/local/etc/poudriere.d/13_2amd64-default-options
[00:00:00] Using ports from: /usr/local/poudriere/ports/default
[00:00:00] Appending to make.conf: /usr/local/etc/poudriere.d/make.conf
[00:00:00] Creating the reference jail... done
[00:00:01] Mounting system devices for 13_2amd64-default
[00:00:01] Mounting ccache from: /var/cache/ccache
[00:00:01] Mounting ports from: /usr/local/poudriere/ports/default
[00:00:01] Mounting packages from: 
[00:00:01] Mounting distfiles from: /data/usr/ports/distfiles/
[00:00:01] Copying /var/db/ports from: /usr/local/etc/poudriere.d/13_2amd64-default-options
[00:00:01] Appending to make.conf: /usr/local/etc/poudriere.d/make.conf
/etc/resolv.conf -> /usr/local/poudriere/data/.m/13_2amd64-default/ref/etc/resolv.conf
[00:00:01] Starting jail 13_2amd64-default
[00:00:02] Ports supports: FLAVORS SELECTED_OPTIONS
[00:00:02] Error: No such port: databases/sql-workbench

poudriere: 3.3.7_4 from the repos works as expected.

mfechner commented 7 months ago

I have a similar problem, to "fix" it I downgraded from poudriere-devel to poudriere

mvanbaak commented 7 months ago

This was fixed in https://github.com/freebsd/poudriere/commit/8726f468060a8886cdfac6d20cae4c997f735406 (I updated the poudriere-devel port to this commit, and can confirm it is working correctly now)

you can also (manually) apply that commit/patch to /usr/local/share/poudriere/options.sh if you need it

mattxtaz commented 7 months ago

Doesn't seem to help me. This is actually the first thing I tried, as I saw this commit, but raised this issue after finding it didn't change anything:

% fetch -o /usr/local/share/poudriere/options.sh https://raw.githubusercontent.com/freebsd/poudriere/8726f468060a8886cdfac6d20cae4c997f735406/src/share/poudriere/options.sh /usr/local/share/poudriere/options.sh 7434 B 7831 kBps 00s % poudriere options -n -f /usr/local/etc/poudriere.d/optlist [00:00:00] Working on options directory: /usr/local/etc/poudriere.d/options [00:00:00] Using ports from: /usr/ports [00:00:00] Appending to make.conf: /usr/local/etc/poudriere.d/make.conf [00:00:00] Error: _jget: 3 arguments expected: var_return ptname property

mvanbaak commented 7 months ago

% poudriere options -n -f /usr/local/etc/poudriere.d/optlist [00:00:00] Working on options directory: /usr/local/etc/poudriere.d/options [00:00:00] Using ports from: /usr/ports [00:00:00] Appending to make.conf: /usr/local/etc/poudriere.d/make.conf [00:00:00] Error: _jget: 3 arguments expected: var_return ptname property

Must be because you don't use a jail name.

root@pkg01:/usr/local/etc/poudriere.d # poudriere options -c -j 14Ramd64 -p development -n -f 14Ramd64-development-port-list
[00:00:00] Working on options directory: /usr/local/etc/poudriere.d/14Ramd64-development-options
[00:00:00] Using ports from: /usr/local/poudriere/ports/development
[00:00:01] Appending to make.conf: /usr/local/etc/poudriere.d/make.conf
[00:00:01] Creating the reference jail... done
[00:00:05] Mounting system devices for 14Ramd64-development
[00:00:06] Mounting ccache from: /var/cache/ccache
[00:00:06] Mounting ports from: /usr/local/poudriere/ports/development
[00:00:06] Mounting packages from:
[00:00:06] Mounting distfiles from: /usr/ports/distfiles
[00:00:06] Copying /var/db/ports from: /usr/local/etc/poudriere.d/14Ramd64-development-options
[00:00:06] Appending to make.conf: /usr/local/etc/poudriere.d/make.conf
/etc/resolv.conf -> /usr/local/poudriere/data/.m/14Ramd64-development/ref/etc/resolv.conf
[00:00:06] Starting jail 14Ramd64-development
[00:00:21] Ports supports: FLAVORS SELECTED_OPTIONS

... here it starts showing the options with portoptions...

mattxtaz commented 7 months ago

Correct. If I add -j 140amd64 then it works. So it's fixed one use case, but it should still work with an empty jail name as it did before the recent update.

edit doesn't quite work. It's now asking me to update every ports options, and is using the defaults rather than the ones i set

edit2 oh i see why. the option file changes to 140amd64-options now because i specified the -j name

madpilot78 commented 7 months ago

Looks like there are two issues here. @mattxtaz is using poudriere options without specifying a jail. This is something I did not foresee when I submitted my patch. Maybe it did work in the "old" versions, but the whole options.sh file was broken at some point and was failing to properly set options for flavored ports. My patch tried to address that. I have no idea how to fix the original bug and not make poudriere options require specifying a jail to start.

The other problem reported by @fernape is a bug I introduced with my patch. Poudriere is truing to use a non existent ports tree, and is fixed by 8726f468060a8886cdfac6d20cae4c997f735406, as @mvanbaak observed.

@bapt The poudriere-devel port in the ports tree should be updated to a commit that includes 8726f468060a8886cdfac6d20cae4c997f735406 to avoid hitting that bug again via the official tree port.

I'm not sure what is your opinion about running poudriere options without specifying a jail. With my patch included this is simply not possible, and my opinion is that we should make specifying the jail mandatory. I don't know how to fix the options.sh logic without starting a jail to extract the required information from the ports tree.

fernape commented 7 months ago

Looks like there are two issues here. @mattxtaz is using poudriere options without specifying a jail. This is something I did not foresee when I submitted my patch. Maybe it did work in the "old" versions, but the whole options.sh file was broken at some point and was failing to properly set options for flavored ports. My patch tried to address that. I have no idea how to fix the original bug and not make poudriere options require specifying a jail to start.

The other problem reported by @fernape is a bug I introduced with my patch. Poudriere is truing to use a non existent ports tree, and is fixed by 8726f46, as @mvanbaak observed.

@bapt The poudriere-devel port in the ports tree should be updated to a commit that includes 8726f46 to avoid hitting that bug again via the official tree port.

I'm not sure what is your opinion about running poudriere options without specifying a jail. With my patch included this is simply not possible, and my opinion is that we should make specifying the jail mandatory. I don't

I would not oppose strongly to this, but if we want to test the same option combination in say 12.4amd64, 12.4i386, 13.2amd64 and 14.0amd64, that means 5 puoudriere options :-)

know how to fix the options.sh logic without starting a jail to extract the required information from the ports tree.

madpilot78 commented 7 months ago

I'm not sure what is your opinion about running poudriere options without specifying a jail. With my patch included this is simply not possible, and my opinion is that we should make specifying the jail mandatory. I don't

I would not oppose strongly to this, but if we want to test the same option combination in say 12.4amd64, 12.4i386, 13.2amd64 and 14.0amd64, that means 5 puoudriere options :-)

Not necessarily. You can just symlink all option directories to the same one and just run options once, which is fundamentally the same you are already doing. You'd just have to choose with which jail to run the option.

On the other hand having poudriere pick a random jail, is prone to foot shooting.

Older method was to use the host OS to configure options, but that stopped working at some point, most probably with the addition of SUBPACKAGES support. It is also not completely correct IMHO, because a port could offer different options depending on the OS version, and using the base system is not correct in that case.

mattxtaz commented 7 months ago

If you read the man page for poudriere-options it doesn't include -j in the synopsis at the top, and in the description of -j it specifically states "If given, configure the options only for the given jail" meaning it is optional. That's why I never used this option in the past. I assume if it isn't given then it's meant to loop through all jails, although I only ever have one jail so have never thought about it.

I have no problem with always using the -j option to specify the jail, but if this is mandatory then the man page should be updated and also it should show the usage text rather than an error about a function only having 2 arguments rather than 3.

madpilot78 commented 7 months ago

Can't do it right now, but I will create a PR to modify the man page.

The -j option could be optional with the old options.sh internal workings. But that logic was broken by unrelated changes in poudriere. Now it needs to have a jail. Grabbing one at a pseudo non random logic looks dangerous. So I'd rather make the -j option mandatory.

madpilot78 commented 7 months ago

@bapt ha produced a much better fix to all these issues here: https://github.com/freebsd/poudriere/issues/1083#issuecomment-1848725388

madpilot78 commented 6 months ago

This should have been fixed by 9143ab494773f7a1ba69efc31177d4bbfe51bf7f and imported in the ports tree, even in the stable port.

Can this issue be closed?

mattxtaz commented 6 months ago

Yes. I now get a warning that tells me to specify -j. So I consider that closed. Thanks!