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.

Sign up for GitHub

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

Jump to bottom

zls: move to by-name #290196

Merged
merged 1 commit into from
Nov 8, 2024
Merged

zls: move to by-name #290196

merged 1 commit into from
Nov 8, 2024

Conversation

moni-dz
Copy link
Contributor

@moni-dz moni-dz commented Feb 20, 2024 •
edited
Loading

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

@ofborg ofborg bot added 10.rebuild-darwin: 1-10 10.rebuild-darwin: 1 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux labels Feb 20, 2024
@wr7 wr7 mentioned this pull request Apr 21, 2024
Merged
13 tasks
@FliegendeWurst FliegendeWurst added the 6.topic: darwin Running or building packages on Darwin label Oct 29, 2024
FliegendeWurst
FliegendeWurst requested changes Oct 29, 2024
View reviewed changes
Copy link
Member

@FliegendeWurst FliegendeWurst left a comment

Choose a reason for hiding this comment

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

ofborg fails:

Running phase: buildPhase
zig build flags: -Dcpu=baseline -Doptimize=ReleaseSafe
error: unable to create compilation: DarwinSdkNotFound

@moni-dz moni-dz force-pushed the zls-asi branch 2 times, most recently from 5abb48c to e4d05d8 Compare November 3, 2024 23:40
@moni-dz
Copy link
Contributor Author

moni-dz commented Nov 3, 2024

should work now with the new darwin sdk rework

@moni-dz moni-dz changed the title zls: build with apple_sdk_11_0 zls: build with apple-sdk_11 Nov 3, 2024
@ofborg ofborg bot requested review from 0x5a4 and figsoda November 4, 2024 15:33
@ofborg ofborg bot added 11.by: package-maintainer This PR was created by the maintainer of the package it changes 10.rebuild-linux: 1-10 10.rebuild-linux: 1 and removed 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux labels Nov 4, 2024
@0x5a4
Copy link
Contributor

0x5a4 commented Nov 4, 2024 •
edited
Loading

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 290196

x86_64-linux

⏩ 1 package marked as broken and skipped:
  • zls

Im very confused by this. Maybe apple-sdk only needs to be a dependency on darwin systems?

FliegendeWurst
FliegendeWurst requested changes Nov 4, 2024
View reviewed changes
pkgs/by-name/zl/zls/package.nix Outdated
zig_0_13.hook
];
nativeBuildInputs = [ zig_0_13.hook ];
buildInputs = [ apple-sdk_11 ];
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
buildInputs = [ apple-sdk_11 ];
buildInputs = lib.optionals stdenv.hostPlatform.isDarwin [ apple-sdk_11 ];

Copy link
Member

Choose a reason for hiding this comment

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

And it looks like we will also have to wait for #353702 to be fixed.

@emilazy
Copy link
Member

emilazy commented Nov 5, 2024

apple-sdk_11 is already the default on aarch64-darwin. Are you sure this does anything to fix the build?

@moni-dz
Copy link
Contributor Author

moni-dz commented Nov 6, 2024

apple-sdk_11 is already the default on aarch64-darwin. Are you sure this does anything to fix the build?

seems it builds now without it

@moni-dz moni-dz changed the title zls: build with apple-sdk_11 zls: move to by-name Nov 6, 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 and removed 10.rebuild-darwin: 1 10.rebuild-darwin: 1-10 10.rebuild-linux: 1 10.rebuild-linux: 1-10 labels Nov 6, 2024
0x5a4
0x5a4 approved these changes Nov 8, 2024
View reviewed changes
Copy link
Contributor

@0x5a4 0x5a4 left a comment

Choose a reason for hiding this comment

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

looks good 👍

@emilazy emilazy merged commit e040101 into NixOS:master Nov 8, 2024
38 of 39 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@FliegendeWurst FliegendeWurst FliegendeWurst requested changes

@0x5a4 0x5a4 0x5a4 approved these changes

@emilazy emilazy emilazy approved these changes

@figsoda figsoda Awaiting requested review from figsoda

Assignees
No one assigned
Labels
6.topic: darwin Running or building packages on Darwin 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 11.by: package-maintainer This PR was created by the maintainer of the package it changes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@moni-dz @0x5a4 @emilazy @FliegendeWurst