Now we need packages that actually make use of those flags and test them with structuredAttrs and without:

How about adding some packages to the passthru.tests of the hooks and checking others manually?

• pythonImportsCheck should be tested carefully with one of the many packages with multiple items set. This would likely fail silently with structuredAttrs on master/staging, testing only the first list item, but would test all of them with this branch.

In my experience, pythonImportsCheckHook is what most Python packages on master would stumble upon switching __structuredAttrs = true. Fixing pythonImportsChceckHook alone would make most packages pass the build.

pypaBuildFlags could be tested with pillow-jpls. This should fail with structuredAttrs on master/staging, if I'm not mistaken, but pass with structuredAttrs in this PR.

pillow-jpls has a strange pypaBuildFlags specification containing single quotes, which seems to expect Bash expansion before execution. However, pypaBuildHook does not evaluate the constructed command before execution like pytestCheckHook does. I wonder if pillow-jpls would ever build the way the package author expects.

Cc: @bcdarwin (python3Packages.pillow-jpls maintainer)

Update: python3Packages.pillow-jpls works for both __structuredAttrs = true and structuredAttrs = false, but the build logs of both cases contain the following message:

*** Configuring CMake...
CMake Warning:
  Ignoring extra path from command line:

   "'--preset=sysdeps'"

This line is also in the build log of python3Packages.pillow-jpls on the master branch.

Submitting #352042 to fix this.

python3Packages.pillow-jpls works for both __structuredAttrs = true and structuredAttrs = false, but the build logs of both cases contain the following message:

Right. And with structuredAttrs = true it fails on current master. So the change to pypaBuildFlags works as intended.

How about adding some packages to the passthru.tests of the hooks and checking others manually?

Hm, I think the most valuable part to test for me is, to confirm that a package fails on master currently with structuredAttrs - but then passes with structuredAttrs on this branch. This needs to be tested manually anyway, to confirm the package broke before. So I don't see much point in adding those passthru.tests right now, if the idea is to eventually switch to "build everthing with structuredAttrs by default". Because then, this will be tested already, but we will surely forget to remove the passthru tests.

In my experience, pythonImportsCheckHook is what most Python packages on master would stumble upon switching __structuredAttrs = true. Fixing pythonImportsChceckHook alone would make most packages pass the build.

I don't understand that, yet. Here's what I understand about the challenge with structuredAttrs:

• A nix list of pythonImportsCheckHook would be passed as a bash array.
• The bash code doesn't expect that and accesses $pythonImportsCheckHook only, which will only ever return the first array element.
• In other cases (flags etc.) this could cause a build failure, but for import checks it would just cause fewer import checks to actually run.

So pythonImportsCheckHook is the one that I expect to not cause any failures at all, but instead silently do bad.

So pythonImportsCheckHook is the one that I expect to not cause any failures at all, but instead silently do bad.

I tested it and now I understand what you mean: Because the python code accesses this variable as an environment variable it works without structuredAttrs. But with structuredAttrs this is not an exported environment variable anymore, but just a bash variable - thus python can't read it, and the imports check fails indeed.

I tested it and now I understand what you mean: Because the python code accesses this variable as an environment variable it works without structuredAttrs. But with structuredAttrs this is not an exported environment variable anymore, but just a bash variable - thus python can't read it, and the imports check fails indeed.

Exactly. The previous implementation of pythonImportsCheckHook relies on pythonImportsCheck being passed to the Python code as an environment variable.

Should we merge this in the next couple days?

I was waiting for @ShamrockLee to rebase per responses to review feedback before merging.

Rebased.

LGTM, thanks!