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

python312Packages.primp: 0.6.5 -> 0.12.0 #379881

Merged
merged 1 commit into from
Feb 10, 2025
Merged

python312Packages.primp: 0.6.5 -> 0.12.0 #379881

merged 1 commit into from
Feb 10, 2025
+45 −6

Conversation

drupol
Copy link
Contributor

@drupol drupol commented Feb 6, 2025 •
edited
Loading

Currently failing.

Log: https://gist.github.com/drupol/42513279900b7e94ea7be5a38ac71f40

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/)
  • 25.05 Release Notes (or backporting 24.11 and 25.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
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@shivaraj-bh
Copy link
Member

shivaraj-bh commented Feb 7, 2025 •
edited
Loading

The failure seems to be because rquest (a dependency of primp) depending on boring2 with pq-experimental feature enabled (see https://github.com/0x676e67/rquest/blob/50614a74a02991124cf0a20ba09de993b79e1223/Cargo.toml#L112).

This feature requires a patch on boringssl. In theory, if we can successfully apply the patch on boringssl drv and set env.BORING_BSSL_ASSUME_PATCHED = true; in primp’s derivation, should fix the build?

I tried overriding boringssl, adding the patch from above, but the patch fails.

Edit: I was only partially right here, see the implementation in #379881 (comment) to see the hack that actually worked.

@shivaraj-bh shivaraj-bh mentioned this pull request Feb 9, 2025
Open
@shivaraj-bh
Copy link
Member

Good news! I got python312Packages.primp to build on x86_64-linux.

If you apply shivaraj-bh@9732d36 on top of the changes in this PR, it should work.

That said, this is just a hack—it couples the boringssl patch with primp, which I’m not a fan of. I’d love to hear from others if there’s a better way to do this.

@drupol
Copy link
Contributor Author

drupol commented Feb 9, 2025

Nice, feel free to apply your commit in this PR. We can proceed this way to unblock the situation and improve it later on.

@newAM
Copy link
Member

newAM commented Feb 9, 2025

Good news! I got python312Packages.primp to build on x86_64-linux.

I wasn't able to replicate this. I got a hash mismatch for the patch:

error: hash mismatch in fixed-output derivation '/nix/store/apw5dp8dg7p4xq9r8lx30jpan7c07ydm-boringssl-44b3df6f03d85c901767250329c571db405122d5.patch.drv':
         specified: sha256-lM+2lLvfDHnxLl+OgZ6R8Y4Z6JfA9AiDqboT1mbxmao=
            got:    sha256-GPQ4YGj5PIi8YSSFJ1ReqyNdcQidCYebJMIqELarcFA=

After correcting the hash I got a build failure.

python3.12-primp> error[E0425]: cannot find value `TLSEXT_TYPE_application_settings_new` in crate `ffi`
python3.12-primp>    --> /build/primp-0.12.0-vendor/boring2-4.15.0/src/ssl/mod.rs:572:19
python3.12-primp>     |
python3.12-primp> 572 |         Self(ffi::TLSEXT_TYPE_application_settings_new as u16);
python3.12-primp>     |                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: a constant with a similar name exists: `TLSEXT_TYPE_application_settings`
python3.12-primp>     |
python3.12-primp>    ::: /build/source/target/x86_64-unknown-linux-gnu/release/build/boring-sys2-e56c8ac74d327e1d/out/bindings.rs:3:210396
python3.12-primp>     |
python3.12-primp> 3   | ... 34 ; pub const TLSEXT_TYPE_application_settings : i32 = 17513 ; pub const TLSEXT_TYPE_encrypted_client_hello : i32 = 65037 ; pub cons...
python3.12-primp>     |          ------------------------------------------------ similarly named constant `TLSEXT_TYPE_application_settings` defined here
python3.12-primp> error: could not evaluate constant pattern
python3.12-primp>    --> /build/primp-0.12.0-vendor/boring2-4.15.0/src/ssl/mod.rs:636:13
python3.12-primp>     |
python3.12-primp> 636 |             ExtensionType::APPLICATION_SETTINGS_NEW => Some(24),
python3.12-primp>     |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
python3.12-primp> For more information about this error, try `rustc --explain E0425`.
python3.12-primp> error: could not compile `boring2` (lib) due to 2 previous errors
python3.12-primp> 💥 maturin failed

Perhaps the patch URL is incorrect?

@shivaraj-bh
Copy link
Member

shivaraj-bh commented Feb 9, 2025 •
edited
Loading

Yes, you are right. See the updated patch URL in shivaraj-bh@ecb8545.

I must’ve have changed the URL by mistake and since the hash was still the same, nix used the correct patch regardless.

@shivaraj-bh
Copy link
Member

I just realized that I wasn’t updating vendorHash after changing boringssl’s src, which caused the build to fail.

A tip for anyone trying to fix this:

Set modRoot = “./src”; when overriding boringssl’s attrs to update vendorHash, since the revision of boringssl we are using is from master-with-bazel branch, where go.mod exists in the src subfolder.

@drupol drupol force-pushed the push-vrznpswzqvwx branch from 1900504 to 5847e9c Compare February 9, 2025 16:58
@drupol drupol marked this pull request as ready for review February 9, 2025 17:21
@drupol drupol force-pushed the push-vrznpswzqvwx branch from 5847e9c to fa40f5e Compare February 9, 2025 17:22
@GaetanLepage
Copy link
Contributor

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 379881

x86_64-linux

❌ 2 packages failed to build:
  • open-webui
  • open-webui.dist
✅ 8 packages built:
  • python312Packages.duckduckgo-search
  • python312Packages.duckduckgo-search.dist
  • python312Packages.primp
  • python312Packages.primp.dist
  • python313Packages.duckduckgo-search
  • python313Packages.duckduckgo-search.dist
  • python313Packages.primp
  • python313Packages.primp.dist

aarch64-linux

✅ 8 packages built:
  • python312Packages.duckduckgo-search
  • python312Packages.duckduckgo-search.dist
  • python312Packages.primp
  • python312Packages.primp.dist
  • python313Packages.duckduckgo-search
  • python313Packages.duckduckgo-search.dist
  • python313Packages.primp
  • python313Packages.primp.dist

x86_64-darwin

❌ 10 packages failed to build:
  • open-webui
  • open-webui.dist
  • python312Packages.duckduckgo-search
  • python312Packages.duckduckgo-search.dist
  • python312Packages.primp
  • python312Packages.primp.dist
  • python313Packages.duckduckgo-search
  • python313Packages.duckduckgo-search.dist
  • python313Packages.primp
  • python313Packages.primp.dist

aarch64-darwin

❌ 10 packages failed to build:
  • open-webui
  • open-webui.dist
  • python312Packages.duckduckgo-search
  • python312Packages.duckduckgo-search.dist
  • python312Packages.primp
  • python312Packages.primp.dist
  • python313Packages.duckduckgo-search
  • python313Packages.duckduckgo-search.dist
  • python313Packages.primp
  • python313Packages.primp.dist

1 similar comment
@GaetanLepage
Copy link
Contributor

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 379881

x86_64-linux

❌ 2 packages failed to build:
  • open-webui
  • open-webui.dist
✅ 8 packages built:
  • python312Packages.duckduckgo-search
  • python312Packages.duckduckgo-search.dist
  • python312Packages.primp
  • python312Packages.primp.dist
  • python313Packages.duckduckgo-search
  • python313Packages.duckduckgo-search.dist
  • python313Packages.primp
  • python313Packages.primp.dist

aarch64-linux

✅ 8 packages built:
  • python312Packages.duckduckgo-search
  • python312Packages.duckduckgo-search.dist
  • python312Packages.primp
  • python312Packages.primp.dist
  • python313Packages.duckduckgo-search
  • python313Packages.duckduckgo-search.dist
  • python313Packages.primp
  • python313Packages.primp.dist

x86_64-darwin

❌ 10 packages failed to build:
  • open-webui
  • open-webui.dist
  • python312Packages.duckduckgo-search
  • python312Packages.duckduckgo-search.dist
  • python312Packages.primp
  • python312Packages.primp.dist
  • python313Packages.duckduckgo-search
  • python313Packages.duckduckgo-search.dist
  • python313Packages.primp
  • python313Packages.primp.dist

aarch64-darwin

❌ 10 packages failed to build:
  • open-webui
  • open-webui.dist
  • python312Packages.duckduckgo-search
  • python312Packages.duckduckgo-search.dist
  • python312Packages.primp
  • python312Packages.primp.dist
  • python313Packages.duckduckgo-search
  • python313Packages.duckduckgo-search.dist
  • python313Packages.primp
  • python313Packages.primp.dist

@drupol
Copy link
Contributor Author

drupol commented Feb 9, 2025

Looks like it is broken on darwin... Do you have a log?

@drupol drupol force-pushed the push-vrznpswzqvwx branch from fa40f5e to 6054034 Compare February 9, 2025 19:42
@GaetanLepage
Copy link
Contributor

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 379881

x86_64-linux

✅ 10 packages built:
  • open-webui
  • open-webui.dist
  • python312Packages.duckduckgo-search
  • python312Packages.duckduckgo-search.dist
  • python312Packages.primp
  • python312Packages.primp.dist
  • python313Packages.duckduckgo-search
  • python313Packages.duckduckgo-search.dist
  • python313Packages.primp
  • python313Packages.primp.dist

aarch64-linux

✅ 8 packages built:
  • python312Packages.duckduckgo-search
  • python312Packages.duckduckgo-search.dist
  • python312Packages.primp
  • python312Packages.primp.dist
  • python313Packages.duckduckgo-search
  • python313Packages.duckduckgo-search.dist
  • python313Packages.primp
  • python313Packages.primp.dist

x86_64-darwin

❌ 10 packages failed to build:
  • open-webui
  • open-webui.dist
  • python312Packages.duckduckgo-search
  • python312Packages.duckduckgo-search.dist
  • python312Packages.primp
  • python312Packages.primp.dist
  • python313Packages.duckduckgo-search
  • python313Packages.duckduckgo-search.dist
  • python313Packages.primp
  • python313Packages.primp.dist

aarch64-darwin

❌ 10 packages failed to build:
  • open-webui
  • open-webui.dist
  • python312Packages.duckduckgo-search
  • python312Packages.duckduckgo-search.dist
  • python312Packages.primp
  • python312Packages.primp.dist
  • python313Packages.duckduckgo-search
  • python313Packages.duckduckgo-search.dist
  • python313Packages.primp
  • python313Packages.primp.dist

@drupol
Copy link
Contributor Author

drupol commented Feb 9, 2025

@GaetanLepage can you share the Darwin logs ?

@GaetanLepage
Copy link
Contributor

boring-ssl fails with:

mkdir: cannot create directory 'build': File exists

@drupol
Copy link
Contributor Author

drupol commented Feb 9, 2025

boring-ssl fails with:

mkdir: cannot create directory 'build': File exists

I'm not calling mkdir on build. I would need to get on a Darwin to fix this.

Maybe @NixOS/darwin-core can help here?

@shivaraj-bh
Copy link
Member

boring-ssl fails with:

mkdir: cannot create directory 'build': File exists

I'm not calling mkdir on build. I would need to get on a Darwin to fix this.

It is probably because of the BUILD file: https://github.com/google/boringssl/blob/44b3df6f03d85c901767250329c571db405122d5/BUILD

@drupol
Copy link
Contributor Author

drupol commented Feb 9, 2025

boring-ssl fails with:

mkdir: cannot create directory 'build': File exists

I'm not calling mkdir on build. I would need to get on a Darwin to fix this.

It is probably because of the BUILD file: https://github.com/google/boringssl/blob/44b3df6f03d85c901767250329c571db405122d5/BUILD

If you have an idea on how to fix it, please let me know.

@drupol drupol mentioned this pull request Feb 9, 2025
Merged
13 tasks
@shivaraj-bh
Copy link
Member

boring-ssl fails with:

mkdir: cannot create directory 'build': File exists

I'm not calling mkdir on build. I would need to get on a Darwin to fix this.

It is probably because of the BUILD file: https://github.com/google/boringssl/blob/44b3df6f03d85c901767250329c571db405122d5/BUILD

If you have an idea on how to fix it, please let me know.

{
  # Remove bazel specific build file to make way for build directory
  # This is a problem on Darwin because of case-insensitive filesystem
  preBuild = (lib.optionalString (stdenv.isDarwin) ''
    rm BUILD
  '') + oa.preBuild;
}

@drupol drupol force-pushed the push-vrznpswzqvwx branch from 6054034 to 2370311 Compare February 9, 2025 21:40
@drupol drupol force-pushed the push-vrznpswzqvwx branch from 2370311 to dc7f6c2 Compare February 9, 2025 21:43
@shivaraj-bh
Copy link
Member

#379881 (comment) is still a problem. Let me send a patch updating vendorHash after I fix an error on x86_64-linux.

@drupol
Copy link
Contributor Author

drupol commented Feb 9, 2025

allrighty, I'll continue tomorrow... heading to bed.

@GaetanLepage
Copy link
Contributor

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 379881

x86_64-linux

✅ 10 packages built:
  • open-webui
  • open-webui.dist
  • python312Packages.duckduckgo-search
  • python312Packages.duckduckgo-search.dist
  • python312Packages.primp
  • python312Packages.primp.dist
  • python313Packages.duckduckgo-search
  • python313Packages.duckduckgo-search.dist
  • python313Packages.primp
  • python313Packages.primp.dist

aarch64-linux

✅ 8 packages built:
  • python312Packages.duckduckgo-search
  • python312Packages.duckduckgo-search.dist
  • python312Packages.primp
  • python312Packages.primp.dist
  • python313Packages.duckduckgo-search
  • python313Packages.duckduckgo-search.dist
  • python313Packages.primp
  • python313Packages.primp.dist

x86_64-darwin

✅ 10 packages built:
  • open-webui
  • open-webui.dist
  • python312Packages.duckduckgo-search
  • python312Packages.duckduckgo-search.dist
  • python312Packages.primp
  • python312Packages.primp.dist
  • python313Packages.duckduckgo-search
  • python313Packages.duckduckgo-search.dist
  • python313Packages.primp
  • python313Packages.primp.dist

aarch64-darwin

✅ 10 packages built:
  • open-webui
  • open-webui.dist
  • python312Packages.duckduckgo-search
  • python312Packages.duckduckgo-search.dist
  • python312Packages.primp
  • python312Packages.primp.dist
  • python313Packages.duckduckgo-search
  • python313Packages.duckduckgo-search.dist
  • python313Packages.primp
  • python313Packages.primp.dist

@drupol
Copy link
Contributor Author

drupol commented Feb 10, 2025

@shivaraj-bh anything else to do here?

@shivaraj-bh
Copy link
Member

@drupol I will be committing in a while. Give me an hour

@shivaraj-bh shivaraj-bh force-pushed the push-vrznpswzqvwx branch 2 times, most recently from cb0c4b6 to 3eddd75 Compare February 10, 2025 08:40
@shivaraj-bh
Copy link
Member

Done! One final nixpkgs-review and we should be good to merge

@GaetanLepage
Copy link
Contributor

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 379881

x86_64-linux

✅ 10 packages built:
  • open-webui
  • open-webui.dist
  • python312Packages.duckduckgo-search
  • python312Packages.duckduckgo-search.dist
  • python312Packages.primp
  • python312Packages.primp.dist
  • python313Packages.duckduckgo-search
  • python313Packages.duckduckgo-search.dist
  • python313Packages.primp
  • python313Packages.primp.dist

aarch64-linux

✅ 8 packages built:
  • python312Packages.duckduckgo-search
  • python312Packages.duckduckgo-search.dist
  • python312Packages.primp
  • python312Packages.primp.dist
  • python313Packages.duckduckgo-search
  • python313Packages.duckduckgo-search.dist
  • python313Packages.primp
  • python313Packages.primp.dist

x86_64-darwin

✅ 10 packages built:
  • open-webui
  • open-webui.dist
  • python312Packages.duckduckgo-search
  • python312Packages.duckduckgo-search.dist
  • python312Packages.primp
  • python312Packages.primp.dist
  • python313Packages.duckduckgo-search
  • python313Packages.duckduckgo-search.dist
  • python313Packages.primp
  • python313Packages.primp.dist

aarch64-darwin

✅ 10 packages built:
  • open-webui
  • open-webui.dist
  • python312Packages.duckduckgo-search
  • python312Packages.duckduckgo-search.dist
  • python312Packages.primp
  • python312Packages.primp.dist
  • python313Packages.duckduckgo-search
  • python313Packages.duckduckgo-search.dist
  • python313Packages.primp
  • python313Packages.primp.dist

GaetanLepage
GaetanLepage approved these changes Feb 10, 2025
View reviewed changes
Copy link
Contributor

@GaetanLepage GaetanLepage left a comment

Choose a reason for hiding this comment

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

Well done guys!

@GaetanLepage GaetanLepage merged commit d7f5894 into NixOS:master Feb 10, 2025
26 of 27 checks passed
@drupol drupol deleted the push-vrznpswzqvwx branch February 10, 2025 09:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@GaetanLepage GaetanLepage GaetanLepage approved these changes

Assignees
No one assigned
Labels
6.topic: python 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@drupol @shivaraj-bh @newAM @GaetanLepage