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

test-admin-deploy-var: Don't rely on OSTREE_FEATURES #3184

Merged
merged 3 commits into from
Feb 21, 2024

Conversation

smcv
Copy link
Contributor

@smcv smcv commented Feb 19, 2024

  • tests: Generalize has_gpgme, has_sign_ed25519 into has_ostree_feature

  • tests: Use skip_without_ostree_feature to detect libarchive, composefs

    This avoids false negatives from ostree --version | grep -q ... exiting with failure under set -o pipefail because grep -q can exit as soon as it sees the desired string, leaving ostree --version to be terminated by SIGPIPE next time it writes to stdout.

  • test-admin-deploy-var: Don't rely on OSTREE_FEATURES

    This is set during build-time testing, but unset during "as-installed" tests.

    Resolves: installed-tests fail with 2024.3: test-admin-deploy-var.sh: line 24: OSTREE_FEATURES: unbound variable #3183

Copy link

openshift-ci bot commented Feb 19, 2024

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@smcv
Copy link
Contributor Author

smcv commented Feb 19, 2024

(testing this next, will undraft when tested)

@smcv smcv force-pushed the issue3183 branch 2 times, most recently from 24ea77e to f6f378f Compare February 19, 2024 16:09
@smcv
Copy link
Contributor Author

smcv commented Feb 19, 2024

This was more involved than I had expected!

smcv and others added 3 commits February 19, 2024 21:01
This avoids false negatives from `ostree --version | grep -q ...`
exiting with failure under `set -o pipefail` because `grep -q` can exit
as soon as it sees the desired string, leaving `ostree --version` to be
terminated by `SIGPIPE` next time it writes to stdout.

Signed-off-by: Simon McVittie <[email protected]>
This is set during build-time testing, but unset during "as-installed"
tests.

Resolves: ostreedev#3183
Signed-off-by: Simon McVittie <[email protected]>
Copy link
Member

@jlebon jlebon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice, thanks!

@jlebon jlebon merged commit 255d40d into ostreedev:main Feb 21, 2024
24 checks passed
Comment on lines +702 to +703
# avoid ostree --version being killed with SIGPIPE and exiting with a
# nonzero status under `set -o pipefail`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh man...wow.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I said, more involved than I expected. This PR brought to you by the dept. of Unix arcana.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even though it's eminently useful, I stopped using pipefail years ago after being burned by the same issue in tar. It might be nice to install a SIGPIPE handler in the version printing code since this is likely to affect other ostree consumers.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One thing I've said many times with coworkers in live video chats, but rarely externally is that a nontrivial component of why I still (20+ years later) wake up and log into my computer for this job is that I just really enjoy collaborating with lots of smart people across the internet. Also there's the continual "surprise" factor in computers as in "what did someone figure out how to make them do this year" (and right now it's AI).

Anyways this little bit combines those two - I just want to say thanks @smcv as one of those smart people on the Internet for everything you do 🙏 - and this little bit of pipefail is one of those surprises 😄

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See #3203.

@smcv smcv deleted the issue3183 branch February 27, 2024 14:40
raspbian-autopush pushed a commit to raspbian-packages/ostree that referenced this pull request Oct 1, 2024
Signed-off-by: Simon McVittie <[email protected]>
Forwarded: ostreedev/ostree#3184

Gbp-Pq: Name tests-Generalize-has_gpgme-has_sign_ed25519-into-has_ostr.patch
raspbian-autopush pushed a commit to raspbian-packages/ostree that referenced this pull request Oct 1, 2024
This avoids false negatives from `ostree --version | grep -q ...`
exiting with failure under `set -o pipefail` because `grep -q` can exit
as soon as it sees the desired string, leaving `ostree --version` to be
terminated by `SIGPIPE` next time it writes to stdout.

Signed-off-by: Simon McVittie <[email protected]>
Forwarded: ostreedev/ostree#3184

Gbp-Pq: Name tests-Use-skip_without_ostree_feature-to-detect-libarchiv.patch
raspbian-autopush pushed a commit to raspbian-packages/ostree that referenced this pull request Oct 1, 2024
This is set during build-time testing, but unset during "as-installed"
tests.

Bug: ostreedev/ostree#3183
Signed-off-by: Simon McVittie <[email protected]>
Forwarded: ostreedev/ostree#3184

Gbp-Pq: Name test-admin-deploy-var-Don-t-rely-on-OSTREE_FEATURES.patch
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.

installed-tests fail with 2024.3: test-admin-deploy-var.sh: line 24: OSTREE_FEATURES: unbound variable
4 participants