unofficial mirror of guix-patches@gnu.org 
 help / color / mirror / code / Atom feed
From: Attila Lendvai <attila@lendvai.name>
To: "Ludovic Courtès" <ludo@gnu.org>
Cc: 50814@debbugs.gnu.org
Subject: [bug#50814] [PATCH] guix: git-authenticate: Also authenticate the channel intro commit.
Date: Mon, 18 Oct 2021 15:27:06 +0000	[thread overview]
Message-ID: <HYmFugT4oHoOLRIfzLRZyEtR3aJeuSjmKed_5Okw0HqRj6pP8A6ITOV_6Ut55WzXhtxilgUeVoqlWQXSfRPvi89xdZnUOKiloJlk-MaPF1A=@lendvai.name> (raw)
In-Reply-To: <878ryqbsvk.fsf@gnu.org>

hi Ludo,


> > i ran the test without my fix, and indeed it fails at two points:
>
> Sorry, which test is failing? Is that part of the patches you sent?
>
> I need more context. :-)


i have sent 5 patches. three of them are prefixed with 'test:', and
two of those are test idempotent test infrastructure changes. the
third of them adds a new test that tests git-authenticate. this is the
test that i'm talking about.

if you apply only these 3 test related commits, and run the new test
on the unpatched codebase, then you'll see the two failures that i'm
talking about in my previous mail.

search the test log for 'FAILURE' (the test runs fully, but fails in
case any of the tests fail).

one of the two failures is a more serious issue because a channel
intro commit is accepted while it shouldn't be.


> > > Alright. Please next time open one issue per topic: that’s a good
> > > way to maximize the chances that review happens in a timely fashion.
> > >
> > > :-)
> >
> > can i mark dependencies between issues/patchsets?
> > because all that i could do here is split this into two sets of
> > commits (because of the dependencies between the commits):
> >
> > 1.  the 3 test commits, and
> > 2.  the 2 guix commits.
> >
> > i thought that separating the test that is exhibiting the bug, from
> > the fix that fixes it, would only hinder the process.
>
> Yes, in general it’s best to have the test and the fix in the same
> commit.


i cut the fix and the test in separate commits (but sent them in the
same patchset/issue), so that it's possible to partially apply only
the test commits, and study its behavior on the current codebase.


> However, at this point, I’m not sure which “bug” we’re talking about.
>
> What you described in your initial message is not a bug in my view:
>
> https://issues.guix.gnu.org/50814#28


the bug is described, formally, by the test that i have added (unless
the test itself is wrong, that is). IIRC, i started putting together
this new test to expose the bugs that i have suspected while reviewing
the implementation of git-authenticate, and then to support my effort
to fix them afterwards.

i think the best next-action is for someone qualified to take a look
at the test that i have added, and see if any of the assumptions
encoded in it is wrong. i think i understand this part of the codebase
pretty well now, but i may have erred.

if the test seems to be valid, then proceed to review the rest of the
commits.


> I’m not saying that we should not change anything, but rather that it’s
> not like a simple usability/UX issue.
>
> I hope this makes sense!


yes, it does! actually, i welcome the reluctance to haphazardly apply
patches to this part of the codebase. i was kinda expecting this, and
that's why i have prepared the commits so that the test can be applied
and tried separately.

hope this clarifies the situation,

--
• attila lendvai
• PGP: 963F 5D5F 45C7 DFCD 0A39
--
“Everything can be taken from a man but one thing: the last of the human freedoms—to choose one’s attitude in any given set of circumstances, to choose one’s own way.”
	— Viktor E. Frankl (1905–1997), 'Man's Search for Meaning' (1946)





  reply	other threads:[~2021-10-18 15:28 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-26 10:19 [bug#50814] [PATCH] guix: git-authenticate: Also authenticate the channel intro commit Attila Lendvai
2021-09-26 18:02 ` Leo Famulari
2021-10-09 13:44   ` Ludovic Courtès
2021-10-12 15:17     ` Leo Famulari
2021-09-26 18:14 ` Maxime Devos
2021-09-27 18:01   ` Attila Lendvai
2021-09-27 18:45   ` Attila Lendvai
2021-09-28 10:02     ` Maxime Devos
2021-09-28  1:05 ` [bug#50814] [PATCH 1/4] tests: Smarten up git repository testing framework Attila Lendvai
2021-09-28  1:05   ` [bug#50814] [PATCH 2/4] tests: Move keys into ./tests/keys/ and add a third ed25519 key Attila Lendvai
2021-09-28  1:05   ` [bug#50814] [PATCH 3/4] tests: Add failing test for .guix-authorizations and channel intro Attila Lendvai
2021-09-29 13:58     ` Maxime Devos
2021-09-28  1:05   ` [bug#50814] [PATCH 4/4] guix: git-authenticate: Fix authenticate-repository Attila Lendvai
2021-09-28 16:24 ` [bug#50814] [PATCH 1/5] tests: Smarten up git repository testing framework Attila Lendvai
2021-09-28 16:24   ` [bug#50814] [PATCH 2/5] tests: Move keys into ./tests/keys/ and add a third ed25519 key Attila Lendvai
2021-09-28 16:24   ` [bug#50814] [PATCH 3/5] tests: Add failing test for .guix-authorizations and channel intro Attila Lendvai
2021-09-28 16:24   ` [bug#50814] [PATCH 4/5] guix: Prepare the UI for continuable &warning exceptions Attila Lendvai
2021-09-29 14:13     ` Maxime Devos
2021-09-29 14:50       ` Attila Lendvai
2021-09-29 20:36         ` Maxime Devos
2021-09-29 21:22           ` Attila Lendvai
2021-09-29 22:03             ` Maxime Devos
2021-09-28 16:24   ` [bug#50814] [PATCH 5/5] guix: git-authenticate: Fix authenticate-repository Attila Lendvai
2021-09-29 23:14     ` Maxime Devos
2021-10-09 13:53 ` [bug#50814] [PATCH] guix: git-authenticate: Also authenticate the channel intro commit Ludovic Courtès
2021-10-09 15:31   ` Attila Lendvai
2021-10-12  9:39     ` Ludovic Courtès
2021-10-17 10:09     ` Attila Lendvai
2021-10-18  9:10       ` Ludovic Courtès
2021-10-18 15:27         ` Attila Lendvai [this message]
2021-10-10 14:15 ` [bug#50814] [PATCH] tests: Add test for .guix-authorizations and channel intro Attila Lendvai
2021-10-18 15:57 ` [bug#50814] [PATCH 1/5] tests: Smarten up git repository testing framework Attila Lendvai
2021-10-18 15:57   ` [bug#50814] [PATCH 2/5] tests: Move keys into ./tests/keys/ and add a third ed25519 key Attila Lendvai
2021-10-18 15:57   ` [bug#50814] [PATCH 3/5] guix: Prepare the UI for continuable &warning exceptions Attila Lendvai
2021-10-18 15:57   ` [bug#50814] [PATCH 4/5] guix: git-authenticate: Fix authenticate-repository Attila Lendvai
2021-10-18 15:57   ` [bug#50814] [PATCH 5/5] tests: Add test for .guix-authorizations and channel intro Attila Lendvai
2022-01-10 14:53     ` [bug#50814] [PATCH] guix: git-authenticate: Also authenticate the channel intro commit Ludovic Courtès
2022-04-04  6:47 ` Attila Lendvai

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='HYmFugT4oHoOLRIfzLRZyEtR3aJeuSjmKed_5Okw0HqRj6pP8A6ITOV_6Ut55WzXhtxilgUeVoqlWQXSfRPvi89xdZnUOKiloJlk-MaPF1A=@lendvai.name' \
    --to=attila@lendvai.name \
    --cc=50814@debbugs.gnu.org \
    --cc=ludo@gnu.org \
    /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).