git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 2/2] t5516/t5601: be less strict about the number of credential warnings

From
Taylor Blau <me@ttaylorr.com>
Date
Nov 1, 2022, 20:50 UTC
Message-ID
<Y2GGofvR0ii2pI1x@nand.local>
In-Reply-To
<Y2DOA7uYfkFveAU9@coredump.intra.peff.net>
On Tue, Nov 01, 2022 at 03:42:59AM -0400, Jeff King wrote:
Show 24 quoted lines
> 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/
Previous: Jeff KingNext: Johannes Schindelin via GitGitGadget
Message 21 of 45 in “t5516/t5601: avoid using localhost for failing HTTPS requests”
  1. 0/2 t5516/t5601: avoid using localhost for failing HTTPS requestsJohannes Schindelin via GitGitGadget, Oct 31, 2022
  2. 1/2 t5516/t5601: avoid using `localhost` for failing HTTPS requestsJohannes Schindelin via GitGitGadget, Oct 31, 2022
  3. Ævar Arnfjörð BjarmasonOct 31, 2022
  4. Jeff KingOct 31, 2022
  5. Taylor BlauNov 1, 2022
  6. Jeff KingNov 1, 2022
  7. Jeff KingNov 1, 2022
  8. Jeff KingNov 1, 2022
  9. 1/2 t5516: move plaintext-password tests from t5601 and t5516Jeff King, Nov 1, 2022
  10. Ævar Arnfjörð BjarmasonNov 1, 2022
  11. Jeff KingNov 1, 2022
  12. Taylor BlauNov 1, 2022
  13. 2/2 t5516/t5601: be less strict about the number of credential warningsJeff King, Nov 1, 2022
  14. Ævar Arnfjörð BjarmasonNov 1, 2022
  15. Jeff KingNov 1, 2022
  16. Ævar Arnfjörð BjarmasonNov 1, 2022
  17. Jeff KingNov 1, 2022
  18. Ævar Arnfjörð BjarmasonNov 1, 2022
  19. Junio C HamanoNov 1, 2022
  20. Jeff KingNov 1, 2022
  21. Taylor BlauNov 1, 2022
  22. 2/2 t5516/t5601: be less strict about the number of credential warningsJohannes Schindelin via GitGitGadget, Oct 31, 2022
  23. Jeff KingOct 31, 2022
  24. Taylor BlauNov 1, 2022
  25. Jeff KingNov 1, 2022
  26. fetch: stop emitting duplicate transfer.credentialsInUrl=warn warningsÆvar Arnfjörð Bjarmason, Oct 31, 2022
  27. Taylor BlauNov 1, 2022
  28. Jeff KingNov 1, 2022
  29. Ævar Arnfjörð BjarmasonNov 1, 2022
  30. Taylor BlauNov 1, 2022
  31. Ævar Arnfjörð BjarmasonNov 1, 2022
  32. Taylor BlauNov 2, 2022
  33. 2/2 t5551: be less strict about the number of credential warningsJeff King, Nov 2, 2022
  34. Eric SunshineNov 2, 2022
  35. Jeff KingNov 2, 2022
  36. Eric SunshineNov 2, 2022
  37. Jeff KingNov 2, 2022
  38. Taylor BlauNov 3, 2022
  39. Jeff KingNov 1, 2022
  40. Ævar Arnfjörð BjarmasonNov 1, 2022
  41. Taylor BlauNov 1, 2022
  42. Ævar Arnfjörð BjarmasonNov 1, 2022
  43. Jeff KingNov 2, 2022
  44. Ævar Arnfjörð BjarmasonNov 4, 2022
  45. Jeff KingNov 4, 2022

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.