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

cargo-show-asm tests: Mark add as inline(never) #282351

Merged
1 commit merged into from
Jan 20, 2024
Merged

cargo-show-asm tests: Mark add as inline(never) #282351

1 commit merged into from
Jan 20, 2024

Conversation

pacak
Copy link
Contributor

@pacak pacak commented Jan 20, 2024

Description of changes

Newer version of rustc breaks currently included test, this PR should fix it.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.05 Release Notes (or backporting 23.05 and 23.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

I didn't test it, but I made a similar change in cargo-show-asm repo here: pacak/cargo-show-asm#231

This should fix the failing test that shows up here: #281810


Add a 👍 reaction to pull requests you find important.

@NixOSInfra NixOSInfra added the 12. first-time contribution This PR is the author's first one; please be gentle! label Jan 20, 2024
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux labels Jan 20, 2024
@ghost
Copy link

ghost commented Jan 20, 2024

@ofborg build cargo-show-asm.passthru.tests

@ghost
Copy link

ghost commented Jan 20, 2024

@pacak the commit needs to confirm to the commit convention. eg: cargo-show-asm: Mark add as inline(never). this can be done using git commit --amend or interactive rebase followed by a force push described in step 7 of this guide

newer rustc automagically marks simple functions as inline, this
includes `add` used for tests. Marking it as `inline(never)` makes sure
it's always included so tests can find it
@pacak
Copy link
Contributor Author

pacak commented Jan 20, 2024

Changed it a bit.

@ghost ghost requested review from matthiasbeyer, oxalica and figsoda January 20, 2024 16:03
@matthiasbeyer
Copy link
Contributor

Result of nixpkgs-review pr 282351 run on x86_64-linux 1

@matthiasbeyer
Copy link
Contributor

Hm, nixpkgs-review did not post anything here... weird?

@delroth delroth added the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Jan 20, 2024
@ghost
Copy link

ghost commented Jan 20, 2024

Hm, nixpkgs-review did not post anything here... weird?

that is expected as the change was just in the test and wouldn't trigger a rebuild.

@ghost ghost merged commit b826219 into NixOS:master Jan 20, 2024
23 checks passed
@pacak pacak deleted the patch-5 branch January 20, 2024 17:07
@matthiasbeyer
Copy link
Contributor

ah 🤠 sure...

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux 12.approvals: 1 This PR was reviewed and approved by one reputable person 12. first-time contribution This PR is the author's first one; please be gentle!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants