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

bear: fix #279035

Merged
merged 4 commits into from
Jan 26, 2024
Merged

bear: fix #279035

merged 4 commits into from
Jan 26, 2024

Conversation

DieracDelta
Copy link
Member

@DieracDelta DieracDelta commented Jan 5, 2024

Description of changes

A continuation of #238531. Fixes bear such that that it runs on aarch64-darwin.

Things done

I cherry picked the commits from #238531, then added another commit to remove the broken marker for bear on darwin. I can squash the commits together if that's better.

I tested on aarch64-darwin by running bear when building nix. compile_commands.json was generated and was non-empty.

  • 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.

Add a 👍 reaction to pull requests you find important.

emilazy and others added 3 commits January 5, 2024 18:29
The previous build system patch was incomplete and left the `wrapper.d`
directory empty, leaving Bear unable to pick up any build commands
at all and breaking the functional test suite, which we also weren't
running. Switch to overriding CMake flags instead and add the patches
and dependencies necessary to get the functional tests running to
prevent a future regression.

I've checked that the instructions in Nix's
`doc/manual/src/contributing/hacking.md` work after this change,
which is what started this yak shave in the first place.
@Atemu
Copy link
Member

Atemu commented Jan 6, 2024

I no longer have a setup to use Bear in any meaningful way but the diff LGTM and it runs successfully on aarch64-darwin.

Copy link
Contributor

@thiagokokada thiagokokada left a comment

Choose a reason for hiding this comment

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

It seems the tests are failing in CI, maybe because of timeout issues?

@DieracDelta
Copy link
Member Author

I think some of the tests are broken. Maybe we could disable the tests for now and fix them later. This would let us get a working bear merged. WDYT?

@thiagokokada
Copy link
Contributor

I think some of the tests are broken. Maybe we could disable the tests for now and fix them later. This would let us get a working bear merged. WDYT?

Fine for me.

@Atemu
Copy link
Member

Atemu commented Jan 11, 2024

Are you still going to do that in this PR or in a follow-up?

@DieracDelta
Copy link
Member Author

I believe I've disabled the tests by added doCheck = false. I'm not super familiar with how Cmake works, though. Will wait to see what CI says.

@DieracDelta
Copy link
Member Author

Hmm that doesn't seem to have worked. Does anyone know how to disable those tests? If not, I'll revert the docheck change and let's merge it?

@DieracDelta
Copy link
Member Author

It looks like those cmake flags helped, except on x86_64-darwin. Not sure how to fix that. In any case, this is probably fine to merge.

This was referenced Jan 25, 2024
@siraben
Copy link
Member

siraben commented Jan 26, 2024

Result of nixpkgs-review pr 279035 run on aarch64-darwin 1

1 package built:
  • bear

@siraben siraben merged commit 2f24ff2 into NixOS:master Jan 26, 2024
22 checks passed
Copy link
Contributor

Successfully created backport PR for release-23.11:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants