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

Check for unrecognized *-sys dependencies #1688

Merged
merged 2 commits into from
Nov 18, 2024

Conversation

EliahKagan
Copy link
Member

@EliahKagan EliahKagan commented Nov 18, 2024

This is a regression test for the improvement in 3506afb (#1684) that fixed the unintended libsqlite3-sys dependency of max-pure reported in #1681. This also tries to guard against the introduction of other such crates as max-pure dependencies.

Specifically, this builds on #1682 by adding another step to pure-rust-build -- this one short, and allowed to fail the job (i.e. not continue-on-error) -- that verifies there are no dependencies named like *-sys, other than linux-raw-sys, which is known about.

I've verified in my fork that the new step fails when applied prior to 3506afb (#1684), and passes afterwards.

Edit: I had meant to do things in such a way as to verify that here, too, but I forgot about how checks are actually run as if on a merge commit that would integrate a PR rather than at the tip of the PR (actions/checkout#504), even though that behavior is something I had recently reviewed for something else. I've edited out the misleading details that were inaccurate with respect to upstream checks.

I've also taken this opportunity to improve a shell variable I had somewhat misnamed, in another pure-rust-build step.

This adds another step to `pure-rust-build` that fails -- and fails
the job -- when there are any dependencies named `*-sys` other than
`linux-raw-sys`, which is known about.

(This is independent of the use of C in `ring` -- discussed
in GitoxideLabs#1681, GitoxideLabs#1682, and GitoxideLabs#1684 -- because `ring` is not, and does not
use, a `*-sys` dependency.)

This should fail prior to 3506afb (GitoxideLabs#1684) and pass afterwards.
@EliahKagan EliahKagan marked this pull request as ready for review November 18, 2024 11:39
This uses the variable name `package` rather than `pattern` for the
variable that expands to an argument to `dpkg-query --status`,
since this argument is always treated as a literal package name.

(I had originally named it `pattern` because I had initially been
thinking of using a different search command.)
Copy link
Member

@Byron Byron left a comment

Choose a reason for hiding this comment

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

Thanks a lot for the follow-up!

Indeed, it should be much harder to sneak sqlite (or similar packages) back in now than it was before, where this could easily have happened without being noticed.

@Byron Byron merged commit b06f729 into GitoxideLabs:main Nov 18, 2024
19 checks passed
@EliahKagan EliahKagan deleted the run-ci/impure-next branch November 18, 2024 12:13
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.

2 participants