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

pgadmin: pin flask-babel to fix build failure #214311

Merged
merged 1 commit into from
Feb 3, 2023

Conversation

gador
Copy link
Member

@gador gador commented Feb 3, 2023

Description of changes

pgadmin needs an older version of flask-security-too, which is incompatible with the update of flask-babel from #211654

This commit pins the version.

Note: nixpkgs-review hangs right now. Will test later (But shouldn't have any impact. No packages depend on pgadmin)

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/)
  • 23.05 Release Notes (or backporting 22.11 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.

pgadmin needs an older version of flask-security-too, which
is incompatible with the update of flask-babel.

This commit pins the version.

Signed-off-by: Florian Brandes <[email protected]>
@gador
Copy link
Member Author

gador commented Feb 3, 2023

so nixpkgs-review is broken since #206969.
This PR build pgadmin and pgadmin.tests just fine.
@NickCao could you have a look and maybe merge it?

@gador
Copy link
Member Author

gador commented Feb 3, 2023

This is a semi-automatic executed nixpkgs-review with nixpkgs-review-checks extension. It is checked by a human on a best effort basis and does not build all packages (e.g. lumo, tensorflow or pytorch).

Result of nixpkgs-review pr 214311 run on x86_64-linux 1

1 package built:
  • pgadmin4

@wegank
Copy link
Member

wegank commented Feb 3, 2023

@ofborg build pgadmin4 pgadmin4.passthru.tests

@wegank
Copy link
Member

wegank commented Feb 3, 2023

Result of nixpkgs-review pr 214311 run on aarch64-darwin 1

1 package failed to build:
  • pgadmin4

@gador
Copy link
Member Author

gador commented Feb 3, 2023

Result of nixpkgs-review pr 214311 run on aarch64-darwin 1
1 package failed to build:

* pgadmin4

Did pgadmin work on aarch64-darwin before? I unfortunately have no way to test that

@wegank
Copy link
Member

wegank commented Feb 3, 2023

The darwin failures were propagated from pyodbc, so false alarm.

@NickCao
Copy link
Member

NickCao commented Feb 3, 2023

@ofborg test pgadmin4-standalone

@NickCao
Copy link
Member

NickCao commented Feb 3, 2023

nixosTests.pgadmin4 is failing evaluation.

@gador
Copy link
Member Author

gador commented Feb 3, 2023

Where do you see that?
I see: pgadmin4, pgadmin4.passthru.tests on x86_64-linux — Success
and
nixosTests.pgadmin4-standalone on x86_64-linux — Success

@NickCao
Copy link
Member

NickCao commented Feb 3, 2023

Where do you see that?

Did an local nix build .#nixosTests.pgadmin4
(Sorry for mistaken edit as reply)

@gador
Copy link
Member Author

gador commented Feb 3, 2023

ah, yeah.
That is a known issue.
From the top of the test file:

  Due the the needed parameters a direct call to "nixosTests.pgadmin4" fails
  and needs to be called as "pgadmin4.tests"

This test suite replaces the typical pytestCheckHook function in python
packages. Pgadmin4 test suite needs a running and configured postgresql
server. This is why this test exists.
To not repeat all the python dependencies needed, this test is called directly
from the pgadmin4 derivation, which also passes the currently
used propagatedBuildInputs and any python overrides.
Unfortunately, there doesn't seem to be an easy way to otherwise include
the needed packages here.
Due the the needed parameters a direct call to "nixosTests.pgadmin4" fails
and needs to be called as "pgadmin4.tests"

@NickCao
Copy link
Member

NickCao commented Feb 3, 2023

A hint: for running postgresql server at test time, we now have postgresqlTestHook

@gador
Copy link
Member Author

gador commented Feb 3, 2023

A hint: for running postgresql server at test time, we now have postgresqlTestHook

Cool. I'll have a look

@NickCao NickCao merged commit 5edd519 into NixOS:master Feb 3, 2023
};
nativeBuildInputs = [ ];
format = "setuptools";
outputs = [ "out" ];
Copy link
Member

Choose a reason for hiding this comment

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

What was the error before you overwrite outputs?

Copy link
Member Author

Choose a reason for hiding this comment

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

Well, due to the update of flask-babel there is now:

outputs = [
    "out"
    "doc"
  ];

and the override does not prove doc.
The error message is:
error: builder for '/nix/store/pz3rhdsvc0m0s8in3pqafvbifblbimq4-python3.10-flask-babel-2.0.0.drv' failed to produce output path for output 'doc' at '/nix/store/pz3rhdsvc0m0s8in3pqafvbifblbimq4-python3.10-flask-babel-2.0.0.drv'

Copy link
Member

Choose a reason for hiding this comment

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

Didn't know that, then it is necessary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants