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

shipwright: patch for asset, data and conf locations #1

Open
wants to merge 1 commit into
base: init/shipwright
Choose a base branch
from

Conversation

PaulGrandperrin
Copy link

No description provided.

@@ -9753,6 +9753,12 @@
githubId = 116740;
name = "Paweł Pacana";
};
paulg = {
Copy link
Owner

@IvarWithoutBones IvarWithoutBones May 26, 2022

Choose a reason for hiding this comment

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

Could you put this in a seperate commit? Formatting for the title should be something like maintainers: add paulg. Then you can add the maintainer addition in shipwright/default.nix in the same commit adding the patches.

@@ -78,6 +79,29 @@ stdenv.mkDerivation rec {
StormLib
];

patches = [
Copy link
Owner

Choose a reason for hiding this comment

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

Could you add a comment explaining these patches are to avoid writing to the nix store / the users current PWD? I know the individual patch titles already explain that, but it'd be nice just for quickly glancing over this code in the future.

Copy link

Choose a reason for hiding this comment

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

Have these patches been proposed upstream?

Choose a reason for hiding this comment

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

Doesn't look like it. They seem to be confined to PaulGrandperrin's personal repository with no corresponding upstream pull requests.

@@ -131,7 +152,7 @@ stdenv.mkDerivation rec {
'';
mainProgram = "soh";
platforms = [ "i686-linux" ];
maintainers = [ maintainers.ivar ];
maintainers = [ maintainers.ivar maintainer.paulg ];
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
maintainers = [ maintainers.ivar maintainer.paulg ];
maintainers = with maintainers; [
ivar
paulg
];

IvarWithoutBones pushed a commit that referenced this pull request Aug 25, 2022
SQLAlchemy-Utils v0.36.6 package override build is failing.

This is due to a patch in the original SQLAlchemy-Utils package which
broke the build of this package override:

```bash
> applying patch /nix/store/pd6anhwbf0in3r3jhi3sbn5v2fjs0mf2-skip-database-tests.patch
> patching file conftest.py
> Hunk #1 FAILED at 61.
> Hunk NixOS#2 succeeded at 98 (offset -10 lines).
```

These SQLAlchemy package overrides were originaly added to fix
incompatibilities with Flask-Admin.

See commit 05ae01f

However with Flask-Admin >= v1.5.6, several SQLAlchemy compatibility patches were added:
* https://flask-admin.readthedocs.io/en/latest/changelog/

We can now safely remove these package overrides to make bukuserver work again.
IvarWithoutBones pushed a commit that referenced this pull request Sep 29, 2022
This reverts commit 246216e.

3.9.1 does not compile due to patch collision:

       > applying patch /nix/store/sickncxw0s730j6gfrnlsi5ndgysi6la-libxml2-cmake-find-package.patch
       > patching file CMakeLists.txt
       > Hunk #1 FAILED at 42.

Fixing it is not trivial as upstream started bundling libxml2:
  NixOS#182941 (comment)

Let's revert the update for now.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants