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

cacert: use buildcatrust build with python3Minimal #176291

Merged
merged 2 commits into from
Jun 7, 2022
Merged

cacert: use buildcatrust build with python3Minimal #176291

merged 2 commits into from
Jun 7, 2022

Conversation

SuperSandro2000
Copy link
Member

@SuperSandro2000 SuperSandro2000 commented Jun 4, 2022 •
edited
Loading

to avoid inifinite recursion with mailcap

Co-authored-by: Artturi Artturin@artturin.com

Closes #176286

Description of changes
Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • 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/)
  • 22.11 Release Notes (or backporting 22.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
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

SuperSandro2000 and others added 2 commits June 4, 2022 22:57
@SuperSandro2000 @Artturin
cacert: use buildcatrust build with python3Minimal
81c57a8 
to avoid inifinite recursion with mailcap

Co-authored-by: Artturi <Artturin@artturin.com>
@Artturin
mailcap: fix build
80f9a78 
and prevent future inf rec issues
@Artturin Artturin mentioned this pull request Jun 4, 2022
Closed
@ofborg ofborg bot requested review from mmahut, WilliButz, globin and kalbasit June 4, 2022 21:30
@ofborg ofborg bot requested review from c0bw3b and adevress June 4, 2022 22:49
@Artturin Artturin merged commit 6919d2e into NixOS:staging Jun 7, 2022
@github-actions github-actions bot mentioned this pull request Jun 7, 2022
Merged
1 task
@github-actions
Copy link
Contributor

github-actions bot commented Jun 7, 2022

Successfully created backport PR #176646 for staging-22.05.

@SuperSandro2000 SuperSandro2000 deleted the cacert-mailcap branch June 10, 2022 00:11
toonn added a commit to toonn/nixpkgs that referenced this pull request Jun 13, 2022
@toonn
cacert: Drop python3Minimal
1e1539c 
This PR, NixOS#176291, started building
`cacert` with `python3Minimal` to work around an infinite recursion
problem with the `mailcap` build.

This causes problems now because `buildcatrust` requires the `_scproxy`
module and that's not built for `python3Minimal` on Darwin. This causes
many transitive failures. Simply building `cacert` with `python3` seems
to work fine.

The `mailcap` expression has since been altered and this infinite
recursion issue doesn't reappear.
@toonn toonn mentioned this pull request Jun 13, 2022
Merged
13 tasks
vcunat pushed a commit that referenced this pull request Jun 15, 2022
@toonn @vcunat
cacert: Drop python3Minimal
ba1d937 
This PR, #176291, started building
`cacert` with `python3Minimal` to work around an infinite recursion
problem with the `mailcap` build.

This causes problems now because `buildcatrust` requires the `_scproxy`
module and that's not built for `python3Minimal` on Darwin. This causes
many transitive failures. Simply building `cacert` with `python3` seems
to work fine.

The `mailcap` expression has since been altered and this infinite
recursion issue doesn't reappear.

(cherry picked from commit 1e1539c)
(effectively a revert of 526a0b8)
@ghost
Copy link

ghost commented Jun 22, 2022

Thank you for rewriting this so it no longer overrides fetcher phases.

I wish it was not allowed to write them that way in the first place....

@Artturin
Copy link
Member

I wish it was not allowed to write them that way in the first place....

I fixed that in #173430

@ghost
Copy link

ghost commented Jun 22, 2022

I fixed that in #173430

this breaks packages which override postFetch e.g. fonts

Awesome. But why did you have to un-fix it?

@Artturin
Copy link
Member

I fixed that in #173430

this breaks packages which override postFetch e.g. fonts

Awesome. But why did you have to un-fix it?

That didn't unfix it

The issue was that postFetch overrode the fetcher postFetch phase and extraPostFetch had to be used
And fixing that broke some packages

@ghost
Copy link

ghost commented Jun 22, 2022

The issue was that postFetch overrode the fetcher postFetch phase and extraPostFetch had to be used
And fixing that broke some packages

Oh, I see.

I was actually wishing that FODs didn't allow any of the phases to be overridden at all. When mantainers can do that it is way too easy for them to accidentally hardwire the current stdenv outpath hash into their expression (example: #171223). Hydra never catches these mistakes because FODs are never rebuilt again. So we end up with stuff in cachix that cannot be recreated after the next commit that changes stdenv or on non x86_64-linux platforms.

I haven't yet seen a case where moving the code from fetchXYZ.postABC into a separate mkDerivation { src = fetchXYZ; buildPhase = postABC; } would cause problems. Maybe there is some use case I'm missing here...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mmahut mmahut Awaiting requested review from mmahut

@WilliButz WilliButz Awaiting requested review from WilliButz

@globin globin Awaiting requested review from globin

@kalbasit kalbasit Awaiting requested review from kalbasit

@c0bw3b c0bw3b Awaiting requested review from c0bw3b

@adevress adevress Awaiting requested review from adevress

Assignees
No one assigned
Labels
10.rebuild-darwin: 501+ 10.rebuild-darwin: 5001+ 10.rebuild-linux: 501+ 10.rebuild-linux: 5001+
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@SuperSandro2000 @Artturin