git guix checkout automation for contributors

git guix checkout automation for contributors

[Danny Milosavljevic]

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

Hi,

So there were some comments on https://news.ycombinator.com/item?id=32960308
about getting started with contributing to Guix.

It turns out that the barrier of entry for editing existing packages is
kinda high (I tried it myself--putting myself into the mindset of a
beginner).

For example:

(1) Install the guix system
(2) Log in as regular user
(3) guix edit nano
(4) It opens a file from /gnu/store that you cannot edit.

Uhh. That's... not ideal. Maybe instead error out with a hint on what you need
to do in order to provide a local checkout? Or do we want viewing to work?

Ok, so let's say we are a little more knowledgeable:

(1) Install the guix system
(2) Log in as regular user
(3a) if [ ! -d src ]
  then
    mkdir src
    cd src
  else
    cd src
  fi \
  && if [ ! -d guix ]
  then
    git clone --depth=1 https://git.savannah.gnu.org/git/guix.git guix
  else
    (cd guix && git pull --rebase)
  fi \
  && cd guix \
  && guix shell -C -D guix -- ./bootstrap \
  && guix shell -C -D guix -- ./configure --localstatedir=/var --disable-daemon \
  && guix shell -C -D guix -- make -j5 \ <--- takes forever to build
  && ./pre-inst-env guix edit nano \
  && git add gnu/packages/*.scm

Ok, that worked.

But there are a lot of ways that can be done wrong.

Weird way it can be done wrong:

(1) install the guix system
(2) log in as regular user
(3b) git clone --depth=1 https://git.savannah.gnu.org/git/guix.git guix \
  && cd guix \
  && guix shell -C -D guix -- ./bootstrap \
  && guix shell -C -D guix -- ./configure --localstatedir=/var --disable-daemon \
  && ./pre-inst-env guix edit nano

It will open the file in /gnu/store . WTF? I'm pretty sure that that's a bug.

./pre-inst-env does exist--but it does some weird things if you don't
compile beforehand.

Also, can we put a variant of (3a) into a command "guix prepare-edit" or
something?
It has always bothered me how manual and magical-commandy it is otherwise
(what you actually need to do also changed over the years--not good).

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: git guix checkout automation for contributors

[Danny Milosavljevic]

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

Also, this doesn't work:

On Sun, 25 Sep 2022 16:06:06 +0200
Danny Milosavljevic <dannym@scratchpost.org> wrote:

(1) install the guix system
(2) log in as regular user
(3c) git clone --depth=1 https://git.savannah.gnu.org/git/guix.git guix \
   && cd guix \
   && guix shell

Even after doing

echo /home/dannym/....../guix >> /home/dannym/.config/guix/shell-authorized-directories

I get:

guix shell: loading environment from '/home/dannym/backup/g1/guix/guix.scm'...
Backtrace:
          14 (primitive-load "/home/dannym/.config/guix/current/bin/?")
In guix/ui.scm:
   2263:7 13 (run-guix . _)
  2226:10 12 (run-guix-command _ . _)
In ice-9/boot-9.scm:
  1752:10 11 (with-exception-handler _ _ #:unwind? _ # _)
  1752:10 10 (with-exception-handler _ _ #:unwind? _ # _)
In guix/store.scm:
   656:37  9 (thunk)
   1295:8  8 (call-with-build-handler #<procedure 7fd26be46e40 at g?> ?)
In guix/status.scm:
    815:4  7 (call-with-status-report _ _)
In guix/scripts/environment.scm:
    320:4  6 (_)
In srfi/srfi-1.scm:
   673:15  5 (append-map _ _ . _)
   586:29  4 (map1 _)
   586:17  3 (map1 ((load package "/home/dannym/backup/g1/guix/?") ?))
In guix/scripts/environment.scm:
    306:4  2 (packages->outputs _ _)
In ice-9/boot-9.scm:
  1685:16  1 (raise-exception _ #:continuable? _)
  1685:16  0 (raise-exception _ #:continuable? _)

ice-9/boot-9.scm:1685:16: In procedure raise-exception:
Throw to key `match-error' with args `("match" "no matching pattern" #<unspecified>)'.

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: git guix checkout automation for contributors

[Maxime Devos]

[-- Attachment #1.1.1: Type: text/plain, Size: 2583 bytes --]


On 25-09-2022 17:04, Danny Milosavljevic wrote:
> Also, this doesn't work:
> 
> On Sun, 25 Sep 2022 16:06:06 +0200
> Danny Milosavljevic<dannym@scratchpost.org>  wrote:
> 
> (1) install the guix system
> (2) log in as regular user
> (3c) git clone --depth=1https://git.savannah.gnu.org/git/guix.git  guix \
>     && cd guix \
>     && guix shell

This issue had been reported in <https://issues.guix.gnu.org/50960#13>:

Liliana Marie Prikler wrote:
> What would happen on the top-level of the Guix source tree or deep
> inside the tree of a guile package that deals with manifests, that
> aren't necessarily related to Guix?  I think we should try searching
> for something less ambiguous first (".guix-shell/manifest" perhaps?)
> and maybe also provide further options after manifest.scm (e.g. build-
> aux/guix.scm or etc/guix.scm)

This issue was considered unlikely:

Ludovic Courtès wrote:
> You mean a directory that contains a file named ‘guix.scm’ or
> ‘manifest.scm’ but that happens to do something completely unrelated?
> 
> We can never rule this out, but I’d say it’s unlikely (these two
> conventions are rather well established) and it’s up to the user to pay
> attention.
> 
> WDYT?

Later shown to be incorrect on the top-level of the Guix source tree 
more explicitly:

Maxime Devos wrote:
> Guix itself doesn't follow this convention: the guix source tree has an unrelated
> "guix.scm" file, that doesn't evaluate to a package.  I'd expect that running
> "guix shell" within the guix source tree would be equivalent to
> "guix environment guix", but currently this doesn't seem to be the case.

As such, it was proposed to leave out auto-detection of ‘guix.scm’:

Liliana Marie Prikler wrote: > Leaving out the auto-detection of 
‘guix.scm’ and ‘manifest.scm’ for now
> is a good idea, given that Guix itself would be an edge-case for that.
> [ proposed alternative solution ]

It was agreed that auto-detection fails here:

Ludovic Courtès wrote:
>> Guix itself doesn't follow this convention: the guix source tree has an unrelated
>> "guix.scm" file, that doesn't evaluate to a package.
> 
> Indeed, but that’s because Guix predates the convention.  :-)
> 
> Toggle quote (4 lines)
>> I'd expect that running "guix shell" within the guix source tree would
>> be equivalent to "guix environment guix", but currently this doesn't
>> seem to be the case.
> 
> Right, it would fail poorly in this particular case.

... and then ignored, AFAICT.

Greetings,
Maxime.

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 929 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

Re: git guix checkout automation for contributors

[Ludovic Courtès]

Hi!

Danny Milosavljevic <dannym@scratchpost.org> skribis:

> (1) Install the guix system
> (2) Log in as regular user
> (3) guix edit nano
> (4) It opens a file from /gnu/store that you cannot edit.

At one point, I thought we could have, say:

  guix edit --clone PKG

It would close the channel containing PKG, open the file that defines
it, and then tell you to run “guix build -L … PKG” to test your changes.

All this would be relatively easy to implement, but it has one downside:
it doesn’t work for the ‘guix’ channel.

Perhaps we could make it work for the ‘guix’ channel if we arrange so
that “guix build -L … PKG” doesn’t end up loading all the local
uncompiled guix/*.scm modules.

Thoughts?

Ludo’.

Re: git guix checkout automation for contributors

[Maxim Cournoyer]

Hi,

Ludovic Courtès <ludo@gnu.org> writes:

> Hi!
>
> Danny Milosavljevic <dannym@scratchpost.org> skribis:
>
>> (1) Install the guix system
>> (2) Log in as regular user
>> (3) guix edit nano
>> (4) It opens a file from /gnu/store that you cannot edit.
>
> At one point, I thought we could have, say:
>
>   guix edit --clone PKG
>
> It would close the channel containing PKG, open the file that defines
> it, and then tell you to run “guix build -L … PKG” to test your changes.
>
> All this would be relatively easy to implement, but it has one downside:
> it doesn’t work for the ‘guix’ channel.
>
> Perhaps we could make it work for the ‘guix’ channel if we arrange so
> that “guix build -L … PKG” doesn’t end up loading all the local
> uncompiled guix/*.scm modules.
>
> Thoughts?

Interesting ideas.

If we decide to implement this, it should work from day one with the
Guix channel, as that's the main use case I believe.

-- 
Thanks,
Maxim

Re: git guix checkout automation for contributors

[zimoun]

Hi,

I am late to the party. :-)

On Sat, 01 Oct 2022 at 19:18, Ludovic Courtès <ludo@gnu.org> wrote:
> Danny Milosavljevic <dannym@scratchpost.org> skribis:
>
>> (1) Install the guix system
>> (2) Log in as regular user
>> (3) guix edit nano
>> (4) It opens a file from /gnu/store that you cannot edit.
>
> At one point, I thought we could have, say:
>
>   guix edit --clone PKG
>
> It would close the channel containing PKG, open the file that defines
> it, and then tell you to run “guix build -L … PKG” to test your changes.
>
> All this would be relatively easy to implement, but it has one downside:
> it doesn’t work for the ‘guix’ channel.
>
> Perhaps we could make it work for the ‘guix’ channel if we arrange so
> that “guix build -L … PKG” doesn’t end up loading all the local
> uncompiled guix/*.scm modules.

Well, does it need to be a Git clone?  Or can it just be a simple copy?

Because using a simple copy, we could imagine,

    guix edit --from=PKG --to=DIRECTORY

that extracts the PKG package definition, filling the required
’#:use-module’s and so ready for “guix build -L DIRECTORY PKG”.

On the other hand, a Git clone would allow to prepare a patch almost
ready for submission.


Cheers,
simon

Re: git guix checkout automation for contributors

[kiasoc5]

Isn't there a checkout cache in ~/.cache/guix? Can guix edit be pointed
to the git repo in the cache as opposed to the system's?

Re: git guix checkout automation for contributors

[zimoun]

Hi,

On dim., 25 sept. 2022 at 21:40, kiasoc5 <kiasoc5@disroot.org> wrote:
> Isn't there a checkout cache in ~/.cache/guix? Can guix edit be pointed
> to the git repo in the cache as opposed to the system's?

This checkout* is used by “guix time-machine” and it does not always
point to what “guix describe” returns.  Modulo this minor warning
(easily fixable by moving HEAD to the correct commit), it appears to me
a good idea to rely on it for easing some contributions.

*checkout:
~/.cache/guix/checkouts/pjmkglp4t7znuugeurpurzikxq3tnlaywmisyr27shj7apsnalwq/


For instance,

--8<---------------cut here---------------start------------->8---
$ guix describe
Generation 9	août 31 2022 14:51:40	(current)
  guix 23152ff
    repository URL: https://git.savannah.gnu.org/git/guix.git
    branch: master
    commit: 23152ff70f0ed4966d8207846f54c793d7cb4f86

$ git -C /home/simon/.cache/guix/checkouts/pjmkglp4t7znuugeurpurzikxq3tnlaywmisyr27shj7apsnalwq/ log -1 --oneline
2e8b4f9bfa (HEAD -> master) installer: Exit console-services page with abort-to-prompt.

$ guix time-machine --url=file:///home/simon/.cache/guix/checkouts/pjmkglp4t7znuugeurpurzikxq3tnlaywmisyr27shj7apsnalwq/ -- describe
  guix 2e8b4f9
    repository URL: https://git.savannah.gnu.org/git/guix.git
    commit: 2e8b4f9bfa00489fd3acff305837a79af236e183
--8<---------------cut here---------------end--------------->8---

When running “guix time-machine” (inferiors), all the machinery
“./bootstrap && ./configure --localstatevar=/var && make” is
transparently done.  We discussed that at the 10 Years event with
Josselin.


Therefore, we could imagine something similar for helping people to
contribute:

 1. use (test?) unmerged patches
 2. update and/or create new patches


Cheers,
simon

Re: git guix checkout automation for contributors

[Josselin Poiret]

Hello everyone,

zimoun <zimon.toutoune@gmail.com> writes:

> Hi,
>
> On dim., 25 sept. 2022 at 21:40, kiasoc5 <kiasoc5@disroot.org> wrote:
>> Isn't there a checkout cache in ~/.cache/guix? Can guix edit be pointed
>> to the git repo in the cache as opposed to the system's?
>
> This checkout* is used by “guix time-machine” and it does not always
> point to what “guix describe” returns.  Modulo this minor warning
> (easily fixable by moving HEAD to the correct commit), it appears to me
> a good idea to rely on it for easing some contributions.

It would be a good idea to reuse the cache, however I don't think that
edits should be made directly there.  Maybe we could either suggest or
provide a command that would copy the git repository there to another
place.

> When running “guix time-machine” (inferiors), all the machinery
> “./bootstrap && ./configure --localstatevar=/var && make” is
> transparently done.  We discussed that at the 10 Years event with
> Josselin.

Yes!  Although FTR none of bootstrap, configure or make are actually run
when using time-machine, it's a totally different way of building :).
The upshot is that we could even provide a flag like `guix time-machine
--issue=NNNNN` that would provide the corresponding guix branch at
https://git.guix-patches.cbaines.net/guix-patches/, and benefit from the
built substitutes if there are.  It would need some more formal URL and
discussion around this feature, but everything's in place for it to
happen.

> Therefore, we could imagine something similar for helping people to
> contribute:
>
>  1. use (test?) unmerged patches
>  2. update and/or create new patches

I think the second part would need a bit of work though, refer to Chris'
mail [1] for what would need to be done (namely, making `git send-email`
easier and refactoring the contribution guidelines).

[1] 87a66wfzeh.fsf@cbaines.net
    https://yhetil.org/guix-devel/87a66wfzeh.fsf@cbaines.net

Best,
-- 
Josselin Poiret

Re: git guix checkout automation for contributors

[zimoun]

Hi,

On Mon, 26 Sep 2022 at 22:37, Josselin Poiret <dev@jpoiret.xyz> wrote:

>> When running “guix time-machine” (inferiors), all the machinery
>> “./bootstrap && ./configure --localstatevar=/var && make” is
>> transparently done.  We discussed that at the 10 Years event with
>> Josselin.
>
> Yes!  Although FTR none of bootstrap, configure or make are actually run
> when using time-machine, it's a totally different way of building :).

Yeah, I simplified since it is somehow a detail. :-) Interested reader
can read Josselin’s explanations [1] about the differences.

1: <https://yhetil.org/guix/87let6roxo.fsf@jpoiret.xyz>

> The upshot is that we could even provide a flag like `guix time-machine
> --issue=NNNNN` that would provide the corresponding guix branch at
> https://git.guix-patches.cbaines.net/guix-patches/, and benefit from the
> built substitutes if there are.  It would need some more formal URL and
> discussion around this feature, but everything's in place for it to
> happen.

Well, I think it would be better to have another subcommand.  There is a
“guix review” draft somewhere but I do not currently find it.

It could be nice to see if it already just work using ’guix time-machine
-C channels.scm’ where the file channels.scm points to some QA.


>> Therefore, we could imagine something similar for helping people to
>> contribute:
>>
>>  1. use (test?) unmerged patches
>>  2. update and/or create new patches
>
> I think the second part would need a bit of work though, refer to Chris'
> mail [1] for what would need to be done (namely, making `git send-email`
> easier and refactoring the contribution guidelines).

I do not think git-send-email is the major roadblock here.  From what I
have seen two steps appear more difficult for infrequent contributors:

 a) commit message
 b) correct format-patch

About a) it could be nice to have a tool roughly checking.  For
instance, we have Emacs yasnippets for adding packages; we could imagine
a tool that check is the commit message respect some standards; at least
help to detect many common “mistakes“.

About b), from the feedback I get, people used to Merge/Pull Request do
not get at first why this step is required; since they usually git-pull
and send a request.  Then, many patches do not contain some base-commit
information because people miss the option --base; implicitly
self-contained with MR/PR.  Idem  the missing --reroll-count.


Cheers,
simon