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

echidna: fix build on aarch64-darwin #331080

Merged
merged 2 commits into from
Jul 31, 2024
Merged

Conversation

alexfmpe
Copy link
Member

Description of changes

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.11 Release Notes (or backporting 23.11 and 24.05 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.

@ofborg ofborg bot added the 6.topic: darwin Running or building packages on Darwin label Jul 30, 2024
@ofborg ofborg bot requested review from arcz and hellwolf July 30, 2024 14:19
@hellwolf
Copy link
Contributor

Hi,

Thanks for the update!

It seems it changed the indentation completely, was that intentional? I tried to run nixfmt, it seems accepting both versions (yours and the one before). If not necessary, I think we shouldn't create such a big formating change.

@alexfmpe
Copy link
Member Author

It seems it changed the indentation completely, was that intentional? I tried to run nixfmt, it seems accepting both versions (yours and the one before). If not necessary, I think we shouldn't create such a big formating change.

Well, I saw CI fail on nixfmt so ran the provided command which produced the second commit.
I don't have an opinion myself either way.

@hellwolf
Copy link
Contributor

Oh, I see, I used nixfmt-rfc-style and ran it when I edited previous file.

Did you also use nixfmt-rfc-style, and how did you run it?

@alexfmpe
Copy link
Member Author

I did nix-shell --run "nixfmt" on the root. This is the message I saw

if (( "${#unformattedFiles[@]}" > 0 )); then
echo "Some new/changed Nix files are not properly formatted"
echo "Please run the following in \`nix-shell\`:"
echo "nixfmt ${unformattedFiles[*]@Q}"
exit 1
fi

@hellwolf
Copy link
Contributor

I can't reproduce it. When I run nixfmt to the old file, nothing changed. I am using latest nixpkgs-unstable nixfmt-rfc-style.

I am starting to suspect that the ci workflow is using a different version of nixfmt, as suggested by these comments:

        run: |
          # pin to a commit from nixpkgs-unstable to avoid e.g. building nixfmt
          # from staging
          # This should not be a URL, because it would allow PRs to run arbitrary code in CI!
          rev=$(jq -r .rev ci/pinned-nixpkgs.json)
          echo "url=https://github.com/NixOS/nixpkgs/archive/$rev.tar.gz" >> "$GITHUB_ENV"

Anyways, I won't hold this up for this thing. Let's just merge the fix!

@maralorn maralorn merged commit 2a5bfea into NixOS:haskell-updates Jul 31, 2024
29 of 31 checks passed
@maralorn
Copy link
Member

Thx

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.

3 participants