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

Avoid generating invalid USE flag combinations #57

Closed
wants to merge 1 commit into from
Closed

Avoid generating invalid USE flag combinations #57

wants to merge 1 commit into from

Conversation

ghost
Copy link

@ghost ghost commented Jan 2, 2019

Consider ignored (through ignoreprefix) USE flags that are enabled on
the system as "always on" in REQUIRED_USE checks of possible
combinations. This makes us avoid generating invalid combinations.

For example, on a system with FEATURES=test, a package with test? ( foo bar ) will always generate a USE line with foo bar in it.

Also, fix the call to portage's check_required_use(): it expects a
list, not a string.

fixes #52

Consider ignored (through `ignoreprefix`) USE flags that are enabled on
the system as "always on" in `REQUIRED_USE` checks of possible
combinations. This makes us avoid generating invalid combinations.

For example, on a system with `FEATURES=test`, a package with `test? (
foo bar )` will always generate a USE line with `foo bar` in it.

Also, fix the call to portage's `check_required_use()`: it expects a
list, not a string.

fixes #52
@ghost ghost mentioned this pull request Jan 2, 2019
@ghost
Copy link
Author

ghost commented Jan 3, 2019

@DerDakon you mentioned earlier that you didn't know much about Python, but you're pretty much the only recent contributor to the project. Who do you think could be in good position, in terms of Python skills and project knowledge, to review this PR?

@DerDakon
Copy link
Contributor

DerDakon commented Jan 3, 2019

I have this patched on my local build machines and will test if it works, that will hopefully be enough.

@DerDakon
Copy link
Contributor

DerDakon commented Jan 3, 2019

This does not set the correct useflags for rdeps, no? You could look on the rdeps of bug 659234 for hppa for the moment.

@ghost
Copy link
Author

ghost commented Jan 4, 2019

Hum, really? From what I understand of the code, the usecombis.py unit is exclusively used for usedeps.sh scripts. The rdeps.sh scripts don't seem to use usecombis.py at all. I don't see much of a difference between rdeps scripts generated on master and on this PR.

@DerDakon
Copy link
Contributor

DerDakon commented Jan 5, 2019

It's no regression, it's just this-is-still-wrong. The rdep script asks the online service for which use flags are needed to test an rdep, but doesn't calculate the flags from there.

@ghost
Copy link
Author

ghost commented Jan 13, 2019

To be sure: are you expecting this PR to solve all outstanding issues?

@DerDakon
Copy link
Contributor

No, surely not. My test is e.g. bug 671044, which has a test for kvazaar, that needs static-libs. Shouldn't that work with this patch? The additional fixes for rdeps are nice to get, but could be in a separate PR. My hope was that it is similar enough to this code that this comes together with this.

@ghost
Copy link
Author

ghost commented Jan 14, 2019

I see what you mean: tatt injects test through --test. If the test USE flag isn't enabled when tatt is ran (when the script is generated), the USE flag combinations aren't going to restrict themselves (because portage, at the time tatt runs, doesn't have test enabled for the target package).

This is a further bug that can be fixed, yes. However, you can verify that this PR fixes something by enabling test for kvazaar on your machine when you run tatt on the very same bug. If you run it on the master branch, you'll see that you get a -static-libs run. If you run it on my PR branch, you'll see that it the -static-libs run is ignored.

@DerDakon
Copy link
Contributor

I can't see a difference between the unpatched and the patched version. Am I missing something essential?

@ghost
Copy link
Author

ghost commented Jan 15, 2019

The difference is that, if you enable FEATURE=test, this PR is going to properly eliminate the -static-libs from bug 671044's run, something that the master branch doesn't do (even if it has FEATURE=test)

@DerDakon
Copy link
Contributor

No, it doesn't, in fact both have no use flags set. Read again or look into the ebuild:

REQUIRED_USE="test? ( static-libs )"

It should add static-libs.

@ghost
Copy link
Author

ghost commented Jan 16, 2019

I know.

My wild guess is that you're not running the version of tatt you think you're running. You could verify by inserting an assert 0 somewhere important: if tatt continues to work, then you're not running the right code. Here's my full output so we can compare:

$ cat /etc/portage/package.env
media-libs/kvazaar test
$ ./env/bin/tatt -b 671044
Bugnumber:  671044
Stabilization bug detected.
Jobname: kvazaar-671044
Found the following package atom : =media-libs/kvazaar-1.2.0-r1
Unmasked =media-libs/kvazaar-1.2.0-r1 in /etc/portage/package.keywords/archtest
  =media-libs/kvazaar-1.2.0-r1: ignoring invalid USE flag combination {'test'}
Success Report script written to kvazaar-671044-success.sh
Commit script written to kvazaar-671044-commit.sh
$ cat kvazaar-671044-useflags.sh
#!/bin/bash
#USE-Flag build tests for job kvazaar-671044

trap "echo 'signal captured, exiting the entire script...'; exit" SIGHUP SIGINT SIGTERM

export TATT_TEST_TYPE="use"
export TATT_REPORTFILE="kvazaar-671044.report"
export TATT_BUILDLOGDIR=""
export TATT_EMERGEOPTS=""

source "/home/vdupras/src/tatt/templates/tatt_functions.sh"

echo -e "USE tests started on $(date)\n" >> kvazaar-671044.report

# Code for =media-libs/kvazaar-1.2.0-r1
 tatt_test_pkg --test "=media-libs/kvazaar-1.2.0-r1"
USE='static-libs' tatt_test_pkg "=media-libs/kvazaar-1.2.0-r1"

echo >> kvazaar-671044.report

Notice the =media-libs/kvazaar-1.2.0-r1: ignoring invalid USE flag combination {'test'} which is the important part that you should get as well.

@DerDakon
Copy link
Contributor

Even then it's wrong: tatt_test_pkg does a build test. It runs FEATURES=test if you pass the option "--test". The result we need and that I have never seen until now would look like this:

USE='static-libs' tatt_test_pkg --test "=media-libs/kvazaar-1.2.0-r1"
USE='static-libs' tatt_test_pkg "=media-libs/kvazaar-1.2.0-r1"
USE='-static-libs' tatt_test_pkg "=media-libs/kvazaar-1.2.0-r1"

@ghost
Copy link
Author

ghost commented Jan 16, 2019

Oh, never mind, I give up.

@ghost ghost closed this Jan 16, 2019
This pull request was closed.
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.

use correct use flags for testing
1 participant