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

treewide: mostly noop: refer to src.name or similar in sourceRoot where appropriate #245388

Merged
merged 5 commits into from
Aug 3, 2023

Conversation

oxij
Copy link
Member

@oxij oxij commented Jul 25, 2023

Description of changes

This replaces all the uses of sourceRoot = "source";, sourceRoot = "source/subdir";, and similar in package derivations that use the default unpackPhase with sourceRoot = src.name, sourceRoot = "${src.name}/subdir"; and similar.

It also fixes sourceRoot and setSourceRoot description in the docs to match the implementation, and deprecates the old usage in release notes.

The primary motivation for this change is to make #49862 (or its reverse) trivial to implement.

Things done

It's a mostly noop change except for the changes to documentation (which cause nixos-install-tools to be rebuilt).

@oxij
Copy link
Member Author

oxij commented Aug 1, 2023

Rebased onto master to fix a conflict with qrcodegen, it will get rebuilt now as 0164461 started to use preBuild for doing the same thing (without good reason, AFAICS).

@oxij
Copy link
Member Author

oxij commented Aug 1, 2023 via email

@drupol
Copy link
Contributor

drupol commented Aug 1, 2023

I did use finalAttrs in the expression that used them already, I was trying to do the minimal change...

I know, but what I'm saying here is a bit different... I'm saying that we should avoid using rec when finalAttrs can be used.

@AndersonTorres
Copy link
Member

The removal of rec can be done in other PRs.

@oxij
Copy link
Member Author

oxij commented Aug 2, 2023

OfBorg seems to be stuck? Pending eval for 20 hours.

@oxij
Copy link
Member Author

oxij commented Aug 2, 2023

@drupol I pushed a new commit that replaces all newly introduced mkDerivation recs with fixpoints. buildRustPackage and buildGoModule do not support this yet, so can't do that there. I consider changing recs that I did not put there to be out of scope of this PR because that's a lot of package-specific work right there. I hope your objection can be withdrawn now?

@drupol
Copy link
Contributor

drupol commented Aug 2, 2023

Excellent, thanks !!!!

@AndersonTorres
Copy link
Member

OfBorg seems to be stuck? Pending eval for 20 hours.

aarch64-darwin usually spends days bootstrapping.
Whereas x86_64-darwin dies of timeout :)

@oxij
Copy link
Member Author

oxij commented Aug 3, 2023

Yay! All checks pass, still no conflicts. A prime opportunity to merge. :)

@AndersonTorres
Copy link
Member

Except merge conflicts.

Maybe because I burned it up :(

@oxij
Copy link
Member Author

oxij commented Aug 3, 2023

Rebased.

@mweinelt mweinelt merged commit 169e4c4 into NixOS:master Aug 3, 2023
6 checks passed
@oxij
Copy link
Member Author

oxij commented Aug 3, 2023

🍻

@oxij
Copy link
Member Author

oxij commented Aug 8, 2023

This now has a continuation in #247977.

@ShamrockLee
Copy link
Contributor

ShamrockLee commented Mar 10, 2024

Please take a look at #294334 for yet another cleanup regarding such hard-coding sourceRoot specification.

This is required for #294068, after which non-compliant packages should fail to build, saving future maintenance overhead.

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.

7 participants