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

[Tracking Issue] __structuredAttrs TODOs and FIXMEs #205690

Open
2 tasks
Artturin opened this issue Dec 11, 2022 · 11 comments
Open
2 tasks

[Tracking Issue] __structuredAttrs TODOs and FIXMEs #205690

Artturin opened this issue Dec 11, 2022 · 11 comments
Labels
0.kind: bug Something is broken 5. scope: tracking Long-lived issue tracking long-term fixes or multiple sub-problems

Comments

@Artturin
Copy link
Member

Artturin commented Dec 11, 2022 •
edited by wolfgangwalther
Loading

Note

Originally started by @Artturin, updated by @ConnorBaker and @wolfgangwalther.

This is a tracking issue to follow progress towards __structuredAttrs compatibility in Nixpkgs. This list is not exhaustive; please feel free to add or comment more issue or PRs you feel are related.

Core

Things relating to adding support for __structuredAttrs to setup hooks or tooling used for ecosystems.

Open

Issues

PRs

TODO

Done

Issues

Nix
Nixpkgs

PRs

Nix
Nixpkgs

Misc

Things relating to __structuredAttrs of some note, like discussions or work suspended until better support for __structuredAttrs is implemented.

Open

PRs

Done

PRs
@Artturin Artturin mentioned this issue Dec 11, 2022
Merged
13 tasks
@Artturin Artturin changed the title structuredAttrs TODOS structuredAttrs TODOs/FIXMEs Dec 11, 2022
@wolfgangwalther
Copy link
Contributor

cmake and meson (and possibly more?) setup hooks are broken: #289037

@tomodachi94 tomodachi94 added the 0.kind: bug Something is broken label May 13, 2024
@ConnorBaker ConnorBaker changed the title structuredAttrs TODOs/FIXMEs [Tracking] __structuredAttrs TODOs and FIXMEs Sep 10, 2024
@ConnorBaker ConnorBaker changed the title [Tracking] __structuredAttrs TODOs and FIXMEs [Tracking Issue] __structuredAttrs TODOs and FIXMEs Sep 10, 2024
@SigmaSquadron SigmaSquadron added the 5. scope: tracking Long-lived issue tracking long-term fixes or multiple sub-problems label Sep 10, 2024
@ShamrockLee
Copy link
Contributor

#347194 enables __structuredAttrs support for Python packages. Please take a look.

@emilazy emilazy mentioned this issue Oct 24, 2024
Merged
13 tasks
@ShamrockLee
Copy link
Contributor

#352709 is a continuation of #339117.

@wolfgangwalther
Copy link
Contributor

structuredAttrs and hardeningEnable don't work together well: #353131

@wolfgangwalther wolfgangwalther mentioned this issue Nov 2, 2024
Closed
13 tasks
@wolfgangwalther
Copy link
Contributor

structuredAttrs and hardeningEnable don't work together well: #353131

Actually fixed in #353142

@wolfgangwalther
Copy link
Contributor

#347194 enables __structuredAttrs support for Python packages. Please take a look.

A part of that is split into #351734.

Ma27 added a commit to Ma27/nixpkgs that referenced this issue Nov 8, 2024
@Ma27
runInLinuxVM: fix for structured attrs
d633c90 
Closes NixOS#334705
Addresses NixOS#205690

The main issue was that the output variable (i.e. `$out` and friends)
didn't exist. I figured the easiest way to add those is to source
`stdenv` here. Given that we build another derivation in this builder,
it's pretty likely that `stdenv` gets pulled already, so I don't expect
a real overhead here.

Also, this mounts `/build` into the VM: this is required to make sure
`.attrs.json` & `.attrs.sh` are available. Dropped the mount of `xchg`
into `/tmp` now since it's also part of `/build`.
@Ma27 Ma27 mentioned this issue Nov 8, 2024
Merged
13 tasks
Ma27 added a commit to Ma27/nixpkgs that referenced this issue Nov 8, 2024
@Ma27
runInLinuxVM: fix for structured attrs
487dd26 
Closes NixOS#334705
Addresses NixOS#205690

The main issue was that the output variable (i.e. `$out` and friends)
didn't exist. I figured the easiest way to add those is to source
`stdenv` here. Given that we build another derivation in this builder,
it's pretty likely that `stdenv` gets pulled already, so I don't expect
a real overhead here.

Also, this mounts `/build` into the VM: this is required to make sure
`.attrs.json` & `.attrs.sh` are available. Dropped the mount of `xchg`
into `/tmp` now since it's also part of `/build`.
@emilazy emilazy mentioned this issue Nov 15, 2024
Merged
13 tasks
This was referenced Nov 18, 2024
Merged
Open
Merged
@wolfgangwalther
Copy link
Contributor

I started to try to build hello with structuredAttrsByDefault.

Before the build starts, output is flooded by eval warnings like this:

warning: In a derivation named 'perl-5.40.0', 'structuredAttrs' disables the effect of the derivation
attribute 'disallowedReferences'; use 'outputChecks.<output>.disallowedReferences' instead

I'm addressing this in #357054.

One of the first failures is glibc trying to append makeFlags like makeFlags="$makeFlags ...", which is not going to be structuredAttrs-safe. Opened #357052 to fix this and many more following same/similar patterns.

One of the next failures is related to substitute / substituteAll not replacing everything they should, because they are missing values. This lead me to refactor how bash builders load their structured attributes consistently in #357053.

This also highlights another problem: substituteAll is incredibly annoying to migrate to structuredAttrs, because missing replacements just go unnoticed. Thus, we should solve #237216 first. For this, we should use replaceVars consistently - which would give us clean error messages when migrating to structuredAttrs, because the required derivation arguments were missing. Also see #339303, which we might need as well.

I can't edit the issue's description, if somebody could add the mentioned issues / PRs, that would be great.

@emilazy emilazy mentioned this issue Nov 18, 2024
Open
@emilazy
Copy link
Member

emilazy commented Nov 18, 2024

I can't edit the issue's description, if somebody could add the mentioned issues / PRs, that would be great.

Yes and yes :)

By the way, I think that we can’t just replace all use of substituteAll with replaceVars, but I’m forgetting why. Something about placeholder?

@wolfgangwalther
Copy link
Contributor

By the way, I think that we can’t just replace all use of substituteAll with replaceVars, but I’m forgetting why. Something about placeholder?

But it's not #339303 that you're thinking of?

@emilazy
Copy link
Member

emilazy commented Nov 18, 2024

Ah, I guess it literally is that :)

Ma27 added a commit to Ma27/nixpkgs that referenced this issue Nov 22, 2024
@Ma27
runInLinuxVM: fix for structured attrs
97ed6b4 
Closes NixOS#334705
Addresses NixOS#205690

The main issue was that the output variable (i.e. `$out` and friends)
didn't exist. I figured the easiest way to add those is to source
`stdenv` here. Given that we build another derivation in this builder,
it's pretty likely that `stdenv` gets pulled already, so I don't expect
a real overhead here.

Also, this mounts `/build` into the VM: this is required to make sure
`.attrs.json` & `.attrs.sh` are available. Dropped the mount of `xchg`
into `/tmp` now since it's also part of `/build`.
@wolfgangwalther
Copy link
Contributor

After #357053 there will be two instances left where $stdenv/setup is sourced without structuredAttrs support. Leaving them as TODO here:

This was referenced Dec 1, 2024
Merged
Merged
@wolfgangwalther wolfgangwalther mentioned this issue Dec 2, 2024
Merged
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
0.kind: bug Something is broken 5. scope: tracking Long-lived issue tracking long-term fixes or multiple sub-problems
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@wolfgangwalther @emilazy @ShamrockLee @Artturin @tomodachi94 @SigmaSquadron