guix-patches
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[bug#50814] [PATCH] guix: git-authenticate: Also authenticate the channe


From: Attila Lendvai
Subject: [bug#50814] [PATCH] guix: git-authenticate: Also authenticate the channel intro commit.
Date: Mon, 18 Oct 2021 15:27:06 +0000

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 via email to

[Prev in Thread] Current Thread [Next in Thread]