Acknowledgement sent
to Sarah Morgensen <iskarian@mgsn.dev>:
New bug report received and forwarded. Copy sent to guix-patches@gnu.org.
(Fri, 03 Sep 2021 04:07:02 GMT) (full text, mbox, link).
Hi Sarah,
On Thu, 02 Sep 2021 at 21:06, Sarah Morgensen <iskarian@mgsn.dev> wrote:
> (define-package my-favorite-package
> (name "my-favorite-package")
> ...)
>
> The purpose is primarily to save the horizontal indent, but IMO it looks
> better, and is marginally more clear for newcomers. I think ideally we could
> eventually transition to using this syntax as the primary syntax and only use
> 'define-public' when necessary.
On one hand, I think it is a good idea; especially for newcomers. On
the other hand, it will break ’git-blame’, isn’t it?
Therefore, I am not convinced such change is worth for ’gnu/packages/’.
Instead it seems worth only for teaching custom packages. Explaining to
people in my labs, they are often confused between ’define’ and
’define-public’. But then, there is two “styles” and people could be
more confused.
Well, my feelings are mixed. Thanks for opening the discussion. :-)
Cheers,
simon
Information forwarded
to guix-patches@gnu.org: bug#50349; Package guix-patches.
(Fri, 03 Sep 2021 21:57:02 GMT) (full text, mbox, link).
Hi,
zimoun <zimon.toutoune@gmail.com> writes:
> Hi Sarah,
>
> On Thu, 02 Sep 2021 at 21:06, Sarah Morgensen <iskarian@mgsn.dev> wrote:
>
>> (define-package my-favorite-package
>> (name "my-favorite-package")
>> ...)
>>
>> The purpose is primarily to save the horizontal indent, but IMO it looks
>> better, and is marginally more clear for newcomers. I think ideally we could
>> eventually transition to using this syntax as the primary syntax and only use
>> 'define-public' when necessary.
>
> On one hand, I think it is a good idea; especially for newcomers. On
> the other hand, it will break ’git-blame’, isn’t it?
Yes, there would be a one-time discontinuity. Reformats like this can
be ignored with the `--ignore-ref' option, or with a file and a config
option:
.git-blame-ignore-revs:
--8<---------------cut here---------------start------------->8---
# Convert 'define-public' forms to 'define-package' forms
15d01b32313f5f2f291b120597719ae92bd26acd
--8<---------------cut here---------------end--------------->8---
.git/config:
--8<---------------cut here---------------start------------->8---
[blame]
ignoreRevsFile = .git-blame-ignore-revs
--8<---------------cut here---------------end--------------->8---
We could include the latter in e.g. a `.gitconfig' file committed to the
repo, but in order to use config settings from the file, users would
have to first run
git config --local include.path ../.gitconfig
Thankfully, this only has to be done once per clone. If you really
wanted to make sure it got run, I suppose you could add it to a 'make'
target.
--
Sarah
Information forwarded
to guix-patches@gnu.org: bug#50349; Package guix-patches.
(Sat, 04 Sep 2021 08:43:01 GMT) (full text, mbox, link).
Sarah Morgensen schreef op do 02-09-2021 om 21:06 [-0700]:
> Hello Guix,
>
> This patch adds a shorthand for "(define-public name (package ...))":
>
> (define-package my-favorite-package
> (name "my-favorite-package")
> ...)
This could be even shorter in the special case that the variable name
and package name are the same (modulo types):
(define-package "my-favorite-package"
(version ...)
...)
'datum->syntax' and 'string->symbol' can be used to turn "my-favorite-package"
into an identifier.
A 'define-unexported-package' might be required in some places.
> The purpose is primarily to save the horizontal indent, but IMO it looks
> better, and is marginally more clear for newcomers. I think ideally we could
> eventually transition to using this syntax as the primary syntax and only use
> 'define-public' when necessary.
>
> There are some downsides... it's one more form to keep track of, and 'let'
> forms can't easily be used with it.
>
> Since it's a syntax rule, it doesn't cause packages to rebuild (tested). I've
> also tested the indentation rules, indent-code.el, and the snippets.
>
> This probably deserves a documentation addition, but I wasn't sure where to
> add it without confusing newcomers. Suggestions welcome!
‘Defining Packages’ would be a good place I think.
> What do you all think?
This looks nice to me. IIUC, the define-package is intended to be clearer
to newcomers, so you might want to ask for feedback on the new syntax on
help-guix@gnu.org.
Greetings,
Maxime.
Severity set to 'wishlist' from 'normal'
Request was from Tobias Geerinckx-Rice <me@tobias.gr>
to control@debbugs.gnu.org.
(Sat, 04 Sep 2021 10:21:01 GMT) (full text, mbox, link).
bug reassigned from package 'guix-patches' to 'guix'.
Request was from Tobias Geerinckx-Rice <me@tobias.gr>
to control@debbugs.gnu.org.
(Sat, 04 Sep 2021 10:21:01 GMT) (full text, mbox, link).
Merged 1528450349.
Request was from Tobias Geerinckx-Rice <me@tobias.gr>
to control@debbugs.gnu.org.
(Sat, 04 Sep 2021 10:21:01 GMT) (full text, mbox, link).
Information forwarded
to bug-guix@gnu.org: bug#50349; Package guix.
(Sat, 04 Sep 2021 10:31:02 GMT) (full text, mbox, link).
All,
To keep a link with previous ‘define-package’ discussion, I've
merged this bug with #15284. It was never resolved IMO and things
have changed since 2013 with the label-less input style.
Maxime Devos 写道:
> This could be even shorter in the special case that the variable
> name
> and package name are the same (modulo types):
>
> (define-package "my-favorite-package"
> (version ...)
> ...)
(define-anything STRING ...) is just too weird to ack. Are there
any package names that aren't currently valid symbols? Is there a
good reason for them?
Kind regards,
T G-R
On 04.09.2021 12:09, Tobias Geerinckx-Rice via Bug reports for GNU Guix wrote:
> All,
>
> To keep a link with previous ‘define-package’ discussion, I've merged this bug with #15284. It was never resolved IMO and things have changed since 2013 with the label-less input style.
>
> Maxime Devos 写道:
>> This could be even shorter in the special case that the variable name
>> and package name are the same (modulo types):
>>
>> (define-package "my-favorite-package"
>> (version ...)
>> ...)
>
> (define-anything STRING ...) is just too weird to ack. Are there any package names that aren't currently valid symbols? Is there a good reason for them?
>
> Kind regards,
>
> T G-R
To me the most obvious thing to do seems
(define-package foo ...) ;no explicit name needed
to bind the variable 'foo' and use symbol->string for the name of the
package, with the possibility to override the name like
(define-package foo (name "foobar") ...)
which would bind the variable 'foo' to a package named "foobar".
Here's a syntax-case definition:
(define-syntax define-package
(lambda (stx)
(syntax-case stx ()
((_ <name>
(<field> <value> ...)
...)
(if (memq 'name (map syntax->datum #'(<field> ...)))
#'(define-public <name>
(package
(<field> <value> ...)
...))
#`(define-public <name>
(package
(name #,(symbol->string (syntax->datum #'<name>)))
(<field> <value> ...)
...)))))))
--
Taylan
Information forwarded
to bug-guix@gnu.org: bug#50349; Package guix.
(Sat, 04 Sep 2021 14:30:02 GMT) (full text, mbox, link).
Information forwarded
to bug-guix@gnu.org: bug#50349; Package guix.
(Sat, 04 Sep 2021 15:02:02 GMT) (full text, mbox, link).
Taylan Kammer 写道:
> To me the most obvious thing to do seems
>
> (define-package foo ...) ;no explicit name needed
>
> to bind the variable 'foo' and use symbol->string for the name
> of the
> package, with the possibility to override the name like
>
> (define-package foo (name "foobar") ...)
>
> which would bind the variable 'foo' to a package named "foobar".
Right, that's what I meant, and it's how I read bug #15284, and it
looks remarkably like the form I use in my personal channels (and
I'm sure I'm not the first! :-).
You're much better at the language/implementation side of things
than I am, Taylan. Would this negatively affect performance
(including ‘guix pull’ compilation)?
Kind regards,
T G-R
On 04.09.2021 16:44, Tobias Geerinckx-Rice wrote:
> Taylan Kammer 写道:
>> To me the most obvious thing to do seems
>>
>> (define-package foo ...) ;no explicit name needed
>>
>> to bind the variable 'foo' and use symbol->string for the name of the
>> package, with the possibility to override the name like
>>
>> (define-package foo (name "foobar") ...)
>>
>> which would bind the variable 'foo' to a package named "foobar".
>
> Right, that's what I meant, and it's how I read bug #15284, and it looks remarkably like the form I use in my personal channels (and I'm sure I'm not the first! :-).
>
> You're much better at the language/implementation side of things than I am, Taylan. Would this negatively affect performance (including ‘guix pull’ compilation)?
>
> Kind regards,
>
> T G-R
I'm flattered, but don't really know the answer, so I decided to attempt
some sort of benchmark. :-P
test1.scm uses the syntax-case macro, test2.scm just define-public.
I don't actually import the Guix modules, so the (package ...) form isn't
macro-expanded, regardless of whether it's used directly or results from
expanding define-package.
This way, the impact of define-package should dominate the time difference.
The results are... interesting. I started out with 256 definitions in the
files, and the define-package one would take about 4.2s to compile while
the regular one about 3.9s. There was some jitter in the results though
after running it several times so I thought, let's increase the number of
packages to reduce noise.
With 512 packages, the one using regular define-public takes a whole
minute to compile, whereas the define-package one just ~14 seconds!
So no idea what's going on there, and how the use of this macro in the
actual (gnu packages ...) modules would affect performance. :-)
--
Taylan
Hi Taylan,
Taylan Kammer <taylan.kammer@gmail.com> writes:
> On 04.09.2021 16:44, Tobias Geerinckx-Rice wrote:
>> Taylan Kammer 写道:
>>> To me the most obvious thing to do seems
>>>
>>> (define-package foo ...) ;no explicit name needed
>>>
>>> to bind the variable 'foo' and use symbol->string for the name of the
>>> package, with the possibility to override the name like
>>>
>>> (define-package foo (name "foobar") ...)
>>>
>>> which would bind the variable 'foo' to a package named "foobar".
>>
>> Right, that's what I meant, and it's how I read bug #15284, and it looks remarkably like the form I use in my personal channels (and I'm sure I'm not the first! :-).
>>
>> You're much better at the language/implementation side of things than I am, Taylan. Would this negatively affect performance (including ‘guix pull’ compilation)?
>>
>> Kind regards,
>>
>> T G-R
I agree; if we added that magic, that's definitely how it should be.
>
> I'm flattered, but don't really know the answer, so I decided to attempt
> some sort of benchmark. :-P
>
> test1.scm uses the syntax-case macro, test2.scm just define-public.
>
> I don't actually import the Guix modules, so the (package ...) form isn't
> macro-expanded, regardless of whether it's used directly or results from
> expanding define-package.
>
> This way, the impact of define-package should dominate the time difference.
>
> The results are... interesting. I started out with 256 definitions in the
> files, and the define-package one would take about 4.2s to compile while
> the regular one about 3.9s. There was some jitter in the results though
> after running it several times so I thought, let's increase the number of
> packages to reduce noise.
>
> With 512 packages, the one using regular define-public takes a whole
> minute to compile, whereas the define-package one just ~14 seconds!
>
> So no idea what's going on there, and how the use of this macro in the
> actual (gnu packages ...) modules would affect performance. :-)
Thanks for running some benchmarks. Were both those latter runs with a
warm cache?
If so, is it possible that due to a compilation optimization, many of
the global symbol lookups only happen once with the define-package
macro?
If you were really interested, I suppose you could test with compilation
optimization off, but I'd be more interested in the performance impact
with (guix packages) imported.
--
Sarah
Information forwarded
to bug-guix@gnu.org: bug#50349; Package guix.
(Sat, 04 Sep 2021 21:02:01 GMT) (full text, mbox, link).
On 04.09.2021 20:53, Sarah Morgensen wrote:
>
> If you were really interested, I suppose you could test with compilation
> optimization off, but I'd be more interested in the performance impact
> with (guix packages) imported.
>
> --
> Sarah
>
Good questions. Let's see. Caching shouldn't be an issue by the way since
I always time a command several times and make sure it's consistent.
With -O0 and -O1, both files take a negligible amount of time to compile,
approximately 0.25s and 0.3s. A difference of 0.5s for 512 packages means
about 0.001s per package, which at 100K packages would be 100s.
That's without importing (guix packages) though. When I import it, then
at -O0 and -O1 (I think these are equivalent), the define-packages one
takes about 3.8s and the regular one about 3.5s. So the difference has
actually shrunk down to about 0.3s now.
With (guix packages) and no special optimization flag, the define-packages
one takes about 26s, and the regular one still shows the strange behavior
where the time explodes to over a minute.
If I remember correctly though, Guix uses -O1 to compile packages anyway.
So all in all I *think* we can say that the macro induces no important
performance hit. (And could for some reason significantly improve it if
we compile a large chunk of packages on -O2...)
One thing I should note though is that I'm using a top-of-the-line typical
consumer CPU (Ryzen 9 3900X) so on an older machine, or a CPU brand that
puts more value into freedom and security than performance, the results
may be different. I still doubt that the impact would be big.
Attached are new scm files since I had to add some fields to make sure
the package macro from (guix packages) doesn't abort the compilation.
--
Taylan
Hi all,
Thanks for your comments. I'm replying specifically to this message but
these thoughts apply to the issue as a whole.
Maxime Devos <maximedevos@telenet.be> writes:
> Sarah Morgensen schreef op do 02-09-2021 om 21:06 [-0700]:
>> Hello Guix,
>>
>> This patch adds a shorthand for "(define-public name (package ...))":
>>
>> (define-package my-favorite-package
>> (name "my-favorite-package")
>> ...)
>
> This could be even shorter in the special case that the variable name
> and package name are the same (modulo types):
>
> (define-package "my-favorite-package"
> (version ...)
> ...)
>
> 'datum->syntax' and 'string->symbol' can be used to turn "my-favorite-package"
> into an identifier.
>
> A 'define-unexported-package' might be required in some places.
Sure, or perhaps 'define-private-package'. I think this is pretty rare,
though? And often in those cases other forms are used which may be
incompatible with the macro, so they'll have to use the original syntax
anyway. Either way is fine IMO.
There are also about 150 packages which use 'package/inherit'. Should
we introduce special syntax for them? What about 'hidden-package'
(about 60 packages)? (And 11 use both!)
About the only form I can think of that wouldn't break the composability
of these kinds of things is something like
(define-package* hello (hidden)
(name "hello")
...)
or
(define-package* hello (hidden inherit-replacements)
(name "hello")
...)
Where 'hidden', 'inherit-replacements', and so on would be procs to
apply (in the same order as compose?) that each transform the package.
Or we could even have them transform the package syntax directly.
But that's even more magic; it would take a fair amount of work, and be
hard to get right. (How well would it hold up to syntax errors?)
>
> [...]
> This looks nice to me. IIUC, the define-package is intended to be clearer
> to newcomers, so you might want to ask for feedback on the new syntax on
> help-guix@gnu.org.
Thanks for the suggestion, I definitely will.
With an eye toward newcomers, I think one "gotcha" of the "optional
name" version is inheritance. If I have
(define-package rust-actix-0.10
(name "rust-actix")
...)
and then I write
(define-package rust-actix-0.20
(inherit rust-actix-0.10)
...)
At best, I would be unsure about whether this package would inherit the
name. At worst, I would assume the name is inherited, and I would be
wrong. If I have to write the name, there is no ambiguity.
For the automatic naming (because of gotchas like that), and for
possible extensions discussed above, I think right now I'm tempted to
agree with Ludo's comment when this last came around (thanks to Tobias
for pointing out the conversation)[0]:
ludo@gnu.org (Ludovic Courtès) writes:
> However, I prefer treating packages just like any other Scheme object,
> and to avoid introducing “magic” with macros like this.
I could be convinced with an elegant enough implementation though! ;)
[0] https://lists.gnu.org/archive/html/bug-guix/2013-09/msg00005.html
--
Sarah
Debbugs is free software and licensed under the terms of the
GNU Public License version 2. The current version can be
obtained from https://bugs.debian.org/debbugs-source/.