git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Taylor Blau <me@ttaylorr.com>
To: Jeff King <peff@peff.net>
Cc: Junio C Hamano <gitster@pobox.com>,
	Johannes Schindelin via GitGitGadget <gitgitgadget@gmail.com>,
	git@vger.kernel.org, Derrick Stolee <derrickstolee@github.com>,
	Johannes Schindelin <johannes.schindelin@gmx.de>
Subject: Re: [PATCH 2/2] t5516/t5601: be less strict about the number of credential warnings
Date: Tue, 1 Nov 2022 16:50:41 -0400	[thread overview]
Message-ID: <Y2GGofvR0ii2pI1x@nand.local> (raw)
In-Reply-To: <Y2DOA7uYfkFveAU9@coredump.intra.peff.net>

On Tue, Nov 01, 2022 at 03:42:59AM -0400, Jeff King wrote:
> On Mon, Oct 31, 2022 at 09:54:23PM -0700, Junio C Hamano wrote:
>
> > Jeff King <peff@peff.net> writes:
> >
> > > It is unclear as to _why_, but under certain circumstances the warning
> > > about credentials being passed as part of the URL seems to be swallowed
> > > by the `git remote-https` helper in the Windows jobs of Git's CI builds.
> > >
> > > Since it is not actually important how many times Git prints the
> > > warning/error message, as long as it prints it at least once, ...
> >
> > Sorry, but I do not quite see the value of keeping the test to
> > expect success in a weakend form.  If we think we are emitting three
> > warnings, whether we do so by mistake or by design, and some of them
> > are lost and not shown for an unknown reason, is there a guarantee
> > that at least one would survive?  And when all three are lost, even
> > the test in the weakened form would fail and stop the CI builds, no?
>
> Without understanding the cause of the loss, I agree that things are a
> little hand-wavy. But the assumption _does_ seem to hold that we
> consistently produce at least one (presumably from the parent
> clone/fetch/push process). And if we can rely on that, there's value in
> the tests asserting that the message was shown to the user at least
> once.

Part of me wonders whether the local DNS-resolution issue you fixed in
the first patch would be sufficient to get us to produce the wrong
number of warnings consistently.

I.e., if I queue just the first patch and drop Johannes's, would that be
sufficient to get CI working consistently again?

I don't know. It's frustrating to rely so much on an external
environment that our feedback loop can only be as short as "push out
some combination of these patches and wait for CI". That's
disappointing, and TBH I would rather spend time focusing on other
patches than play games with CI.

The pair of patches look good to me. Perhaps we could avoid the weakened
assumption, but I do not mind too much in the meantime. Especially since
we already have a series[1] in the works that resolves the issue for
good.

Thanks,
Taylor

[1]: https://lore.kernel.org/git/RFC-patch-1.1-0266485bc6c-20221031T204149Z-avarab@gmail.com/

  reply	other threads:[~2022-11-01 20:50 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-10-31 19:47 [PATCH 0/2] t5516/t5601: avoid using localhost for failing HTTPS requests Johannes Schindelin via GitGitGadget
2022-10-31 19:47 ` [PATCH 1/2] t5516/t5601: avoid using `localhost` " Johannes Schindelin via GitGitGadget
2022-10-31 20:49   ` Ævar Arnfjörð Bjarmason
2022-10-31 23:20   ` Jeff King
2022-11-01  0:59     ` Taylor Blau
2022-11-01  2:28       ` Jeff King
2022-11-01  2:03     ` Jeff King
2022-11-01  2:25       ` Jeff King
2022-11-01  2:26         ` [PATCH 1/2] t5516: move plaintext-password tests from t5601 and t5516 Jeff King
2022-11-01  3:18           ` Ævar Arnfjörð Bjarmason
2022-11-01  7:32             ` Jeff King
2022-11-01 20:37               ` Taylor Blau
2022-11-01  2:26         ` [PATCH 2/2] t5516/t5601: be less strict about the number of credential warnings Jeff King
2022-11-01  3:29           ` Ævar Arnfjörð Bjarmason
2022-11-01  7:39             ` Jeff King
2022-11-01  8:15               ` Ævar Arnfjörð Bjarmason
2022-11-01  9:12                 ` Jeff King
2022-11-01 14:05                   ` Ævar Arnfjörð Bjarmason
2022-11-01  4:54           ` Junio C Hamano
2022-11-01  7:42             ` Jeff King
2022-11-01 20:50               ` Taylor Blau [this message]
2022-10-31 19:47 ` Johannes Schindelin via GitGitGadget
2022-10-31 23:22   ` Jeff King
2022-11-01  0:57     ` Taylor Blau
2022-11-01  2:27   ` Jeff King
2022-10-31 20:47 ` [RFC PATCH] fetch: stop emitting duplicate transfer.credentialsInUrl=warn warnings Ævar Arnfjörð Bjarmason
2022-11-01  1:06   ` Taylor Blau
2022-11-01  2:32   ` Jeff King
2022-11-01  3:01     ` Ævar Arnfjörð Bjarmason
2022-11-01 20:54       ` Taylor Blau
2022-11-01 22:17         ` Ævar Arnfjörð Bjarmason
2022-11-02  0:53           ` Taylor Blau
2022-11-02  8:42         ` [PATCH v3 2/2] t5551: be less strict about the number of credential warnings Jeff King
2022-11-02  8:49           ` Eric Sunshine
2022-11-02  9:15             ` Jeff King
2022-11-02  9:31               ` Eric Sunshine
2022-11-02  9:18           ` Jeff King
2022-11-03  1:31             ` Taylor Blau
2022-11-01  9:35     ` [RFC PATCH] fetch: stop emitting duplicate transfer.credentialsInUrl=warn warnings Jeff King
2022-11-01 13:07       ` Ævar Arnfjörð Bjarmason
2022-11-01 21:00         ` Taylor Blau
2022-11-01 21:57           ` Ævar Arnfjörð Bjarmason
2022-11-02  8:19             ` Jeff King
2022-11-04  9:01               ` Ævar Arnfjörð Bjarmason
2022-11-04 13:16                 ` Jeff King

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: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=Y2GGofvR0ii2pI1x@nand.local \
    --to=me@ttaylorr.com \
    --cc=derrickstolee@github.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=gitster@pobox.com \
    --cc=johannes.schindelin@gmx.de \
    --cc=peff@peff.net \
    /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://80x24.org/mirrors/git.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).