-
-
Notifications
You must be signed in to change notification settings - Fork 13.5k
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
mcomix3: init at 20201123 #104908
mcomix3: init at 20201123 #104908
Conversation
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/packaging-mcomix3-python-gtk-missing-gsettings-schemas-issue/10190/4 |
Result of 1 package built:
|
Result of 1 package built:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now squash all those small commits to a single commit.
Or, more accurately, two:
- the first including yourself in the maintainers list
- the other including all else, namely, the package expression.
Okay, little help with that please, because if I do Edit: I think I managed. It was |
Usually I just
After this, the branch is on par with upstream Nixpkgs. |
But just adding it to buildInputs will not be sufficient. That will
only cause it to be on PATH during the build. If you want it to be
available at runtime as well, you will need to use wrappers.
…On Wed, Nov 25, 2020 at 16:09, Ryan Mulligan ***@***.***> wrote:
@ryantm commented on this pull request.
In pkgs/applications/graphics/mcomix3/default.nix:
> + '';
+
+ # The tests seem to be broken upstream
+ #checkInputs = [ python3Packages.pytest ];
+ #checkPhase = ''
+ # HOME=$TMPDIR pytest
+ #'';
+ # pythonImportsCheck = [ "mcomix" ];
+ doCheck = false;
+
+ meta = with stdenv.lib; {
+ description = "Comic book reader and image viewer; python3 fork
of mcomix";
+ longDescription = "User-friendly, customizable image viewer,
specifically designed to handle comic books and manga supporting a
variety of container formats (including CBR, CBZ, CB7, CBT, LHA and
PDF)";
+ homepage = "https://github.com/multiSnow/mcomix3";
+ changelog =
"https://github.com/multiSnow/mcomix3/blob/gtk3/ChangeLog";
+ license = licenses.gpl2Plus;
@SuperSandro2000 Right, they won't be able to build this.
@con-f-use If unrar is critical to how it works, I'd say leave it how
it is. If not, then your unrarSupport change sounds good.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
Okay, I'll look into wrapping. Is there a good way, so it doesn't interfere with the wrapping from Edit: So, what I'm planing to do is just add which results in wrapping all the things:
Is that okay? @jtojnar |
I think you will need to wrap the things manually. First, nixpkgs/pkgs/applications/misc/genxword/default.nix Lines 39 to 45 in ad8238c
|
Well, I don't seem to need to, because multiple layers of wrapping are working, the environment variables are all there, once I get to the actual mcomix3 executable, but it would be nicer. |
You can just add stuff to But yeah, it is a trade off between having nested wrappers and slightly more complicated package expressions. |
dontWrapGApps = true;
preFixup = ''
makeWrapperArgs+=(
"''${gappsWrapperArgs[@]}"
"--prefix" "PATH" ":" "${lib.makeBinPath ([ p7zip lhasa mupdf ] ++ lib.optional (unrarSupport) unrar)}"
)
''; The
Then the arguments from |
You cannot set What do you mean the prefix does not work? Is it not appearing in the created wrapper? |
Nevermind, I think, I'm finally there. Thanks for your insight! |
Okay, I think I have followed all the suggestions. @jtojnar , please tell me if you now think it's ready to be merged, then I'll squash the commits again, and hopefully be done. |
Yes it looks okay to me now. Thanks. |
Okay, commits are squashed, what needs to happen now to get this thing merged? |
I can merge this once CI finishes. Thanks. |
Thank you, I learned a lot. CI seems finished... |
Thanks. |
Motivation for this change
The original mcomix used to be in nixpkgs, but became unmaintained and is not Python 3 compatible. It was subsequently removed. Debian, Archlinux, Fedora and others have switched to it's actively maintained Python 3 fork, mcomix3. This PR packages mcomix3 for nixpkgs.
It is @con-f-use first contribution to nixpkgs, so apologies for any errors or breach of protocol.
Closes #75028.
Things done
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
./result/bin/
)nix path-info -S
before and after)