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

Build failure: python312Packages.pysaml2 #367976

Open
Guanran928 opened this issue Dec 24, 2024 · 7 comments
Open

Build failure: python312Packages.pysaml2 #367976

Guanran928 opened this issue Dec 24, 2024 · 7 comments
Labels
0.kind: build failure A package fails to build

Comments

@Guanran928
Copy link
Contributor

Steps To Reproduce

Steps to reproduce the behavior:

  1. build python312Packages.pysaml2

Build log

https://pb.ny4.dev/1Mxszc

Additional context

This packages is a dependency of matrix-synapse.

Metadata

  • system: "x86_64-linux"
  • host os: Linux 6.13.0-rc3, NixOS, 25.05 (Warbler), 25.05.20241224.430374d
  • multi-user?: yes
  • sandbox: yes
  • version: nix-env (Nix) 2.24.11
  • nixpkgs: /nix/store/j8dj0k38p10zpnpvi87xfraxygclysbx-source

Notify maintainers


Note for maintainers: Please tag this issue in your PR.


Add a 👍 reaction to issues you find important.

@Guanran928 Guanran928 added the 0.kind: build failure A package fails to build label Dec 24, 2024
@yu-re-ka
Copy link
Contributor

4700074d7d3acfa80840db9c12793de86ae9f9d7 is the first bad commit
commit 4700074d7d3acfa80840db9c12793de86ae9f9d7
Author: Martin Weinelt <[email protected]>
Date:   Wed Dec 4 02:47:50 2024 +0100

    python312Packages.pyopenssl: 24.2.1 -> 24.3.0
    
    https://github.com/pyca/pyopenssl/blob/24.3.0/CHANGELOG.rst

 pkgs/development/python-modules/pyopenssl/default.nix | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

@mweinelt
Copy link
Member

Filed an issue upstream, since we are within their version boundary.

IdentityPython/pysaml2#975

@jcaesar
Copy link
Contributor

jcaesar commented Dec 27, 2024

It seems the fix that upstream wants to do is pending for two years. Waiting for upstream to move on this may be overly optimistic.

Of course, you can skip the relevant tests (and then e.g. also skip some tests in synapse, because that has some failing tests too, currently).

      (final: prev: {
        python312 = prev.python312.overridee { # deliberate typo. don't copy this into your config blindly
          packageOverrides = final: prev: {
            pysaml2 = prev.pysaml2.overridePythonAttrs (orig: {
              disabledTests =
                orig.disabledTests
                ++ [
                  "test_encrypted_response_6"
                  "test_validate_cert_chains"
                  "test_validate_with_root_cert"
                ];
            });
          };
        };
        matrix-synapse-unwrapped = prev.matrix-synapse-unwrapped.overridePythonAttrs {doCheck = false;}; # todo skip right tests
      })

What I don't know is whether this results in a working and secure synapse…

What's the proper way forward here? Downgrade pyopenssl for pysaml2?

@bachp
Copy link
Member

bachp commented Dec 27, 2024

@jcaesar The problem is that it's not only tests that are failing but, the certificate verification is really not working as it is using deprecated functions that got removed.

That said I it seems that saml2 support is an optional feature for matrix-synapse, so we could probably just remove it until upstream is fixed. This way at least everybody who is not using saml2 can continue with a working matrix server.

@artemislena
Copy link

artemislena commented Dec 28, 2024

That said I it seems that saml2 support is an optional feature for matrix-synapse, so we could probably just remove it until upstream is fixed. This way at least everybody who is not using saml2 can continue with a working matrix server.

T.: Any way one could remove the dependency manually w some override (n consequently build the pkg manually, Ig) until that change arrives in nixpkgs?

@jcaesar
Copy link
Contributor

jcaesar commented Dec 30, 2024

@artemislena I sure would like to know that too. I'm currently using the sledgehammer option of stubbing out pysaml2, but I don't understand why its necessary, as pysaml2 is already an optional dependency and I don't have the config option that would add the relevant extra.

For reference, my current overlay:

      (final: prev: {
        python312 = prev.python312.override {
          packageOverrides = final: prev: {
            pysaml2 = final.toPythonModule final.emptyDirectory;
          };
        };
        matrix-synapse-unwrapped = prev.matrix-synapse-unwrapped.overrideAttrs (old: {
          postPatch = (old.postPatch or "") + ''
            substituteInPlace tests/storage/databases/main/test_events_worker.py --replace-fail \
            $'    def test_recovery(' \
            $'    from tests.unittest import skip_unless\n'\
            $'    @skip_unless(False, "broken")\n'\
            $'    def test_recovery('
          '';
        });
      })

(See also #369303)

@xinyangli
Copy link
Contributor

@jcaesar FYI, matrix-synapse-unwrapped add all optional-dependencies to nativeCheckInputs unconditionally, which cause pysaml2 to be a dependency of matrix-synapse. Check out matrix-synapse/default.nix

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0.kind: build failure A package fails to build
Projects
None yet
Development

No branches or pull requests

7 participants