unofficial mirror of guix-patches@gnu.org 
 help / color / mirror / code / Atom feed
From: Christopher Baines <mail@cbaines.net>
To: Ekaitz Zarraga <ekaitz@elenq.tech>
Cc: "44170@debbugs.gnu.org" <44170@debbugs.gnu.org>
Subject: [bug#44170] [PATCH] gnu: Correct Inkscape extension dependencies
Date: Sat, 24 Oct 2020 23:14:27 +0100	[thread overview]
Message-ID: <87r1pn9sqk.fsf@cbaines.net> (raw)
In-Reply-To: <khNSOQlCgQUMMPPih274U1RU9zftDkw7N6UZSIrv__pyPZP9_Nea5N211731pkpI7WIZ-i-YCeM3r0Y7VYvoqM5x1lyLG9lYwdXdhr_e4AE=@elenq.tech>

[-- Attachment #1: Type: text/plain, Size: 4046 bytes --]


Ekaitz Zarraga <ekaitz@elenq.tech> writes:

>> The : after /bin is unnecessary.
>
> I copied all this from Kicad's package because it's a software
> that has a similar python plugin style.

That's fine, it's also unnecessary there :)

If you pull the generated shell out of the bin/kicad or bin/inkscape
file, and play around with it in a shell, you should see something like:

→ export PATH="/gnu/store/11l2qmzfgsp7k345mv6x1vn64q8330kw-python-wrapper-3.8.2/bin:${PATH:+:}$PATH"

→ echo $PATH
/gnu/store/11l2qmzfgsp7k345mv6x1vn64q8330kw-python-wrapper-3.8.2/bin::/gnu/store/hgh67ilwsbx6v2irc7vgrnv354cv4h8a-profile/bin


Note the double colon, I don't think it's a problem in terms of
functionality, but just in case someone copies this from inkscape as an
example, it would be good not to do the same here.

Note, there's no need to fix this in kicad. If you do want to, I'd
submit a separate commit under a seperate bug.

>> Also, it looks like python-wrapper is already referenced lots in the
>> output, did you have a specific reason why wrapping inkscape with the
>> PATH was useful?
>
> Not really.
> I just copied it from kicad and followed the discussion at guix-devel...
> I thought it was necessary but watching what python-wrapper does it's
> probably not necessary. I'll try it without the PATH and remove it if
> it's not useful.
>
> I'll compare with kicad's package too, because they are very similar
> so they should have some relation on this too.

Sounds good, it's possible something, or plugins expect the PATH to have
python on it, if so, that's fine, but it would be good to note the
reason in a comment. Otherwise, this bit can be removed.

>> So, before python-wrapper was a native-input, and you've added some
>> Python packages as native inputs.
>>
>> The distinction for inkscape between an input and a native input is
>> mostly academic at this point, because the meson build system doesn't
>> support cross builds.
>>
>> However, inkscape already uses references python-wrapper in its output,
>> so it should probably be an input. With this change, you're also setting
>> out that inkscape should be able to use these Python libraries at
>> runtime, hence they should be inputs (matching the architecture you're
>> building for), rather than native inputs (matching the architecture
>> you're building on).
>>
>> Does that make sense?
>
> Something obviously doesn't. :)
> In my understanding, python should be an `input` but it was already in
> `native-inputs`, I don't know why. So I considered all the rest of the
> python-related packages should be in the same block.
>
> If I did this package I would add python-* and python itself as inputs.
>
> So, I follow your explanation and it's what I understood, but I don't
> get why were python related things in native-inputs before. That
> confused me.
>
> I'll move those to inputs. Could you explain or find a reason why python
> wasn't an input before? is it just an error?

Well, error is a strong word, in this case, given the build system
doesn't support building for a non-native architecture, it's sort of a
non-issue. But I would view it as something you're fixing in addition to
the changes you're making.

It didn't (and still doesn't to some extent) matter that the Python
stuff was/is in native-inputs, because cross building this package is
impossible because of the build system.

However, in case the build system changes to support this, to be
consistent with other package definitions, and to be consistent with the
general rule that you don't reference native-inputs from the outputs (at
least I think that's a rule or at least strong convention), having the
Python stuff in inputs is better. Even if the package builds just the
same.

> Thank you for your time and for the explanations. Really helpful.
>
> I'll follow up with the updated patch soonish. Only with the Inkscape
> part.

You're welcome :) I look forward to it.

Thanks,

Chris

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 987 bytes --]

  reply	other threads:[~2020-10-24 22:15 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-23 10:32 [bug#44170] [PATCH] gnu: Correct Inkscape extension dependencies Ekaitz Zarraga
2020-10-23 23:03 ` Ekaitz Zarraga
2020-10-24 19:55   ` Christopher Baines
2020-10-24 21:31     ` Ekaitz Zarraga
2020-10-24 22:14       ` Christopher Baines [this message]
2020-10-25 12:23         ` Ekaitz Zarraga
2020-10-26 15:56           ` bug#44170: " Christopher Baines

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://guix.gnu.org/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87r1pn9sqk.fsf@cbaines.net \
    --to=mail@cbaines.net \
    --cc=44170@debbugs.gnu.org \
    --cc=ekaitz@elenq.tech \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).