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

Re: [PATCH] t3070: make chain lint tester happy

From
PWPhillip Wood <phillip.wood123@gmail.com>
Date
Mar 26, 2023, 14:30 UTC
Message-ID
<e240a88d-37d0-83c6-7268-0fc15f5dee18@dunelm.org.uk>
In-Reply-To
<20230325080453.GA852237@coredump.intra.peff.net>
On 25/03/2023 08:04, Jeff King wrote:
Show 20 quoted lines
> On Sat, Mar 25, 2023 at 03:58:32AM -0400, Jeff King wrote:
> 
>>>> This looks like the right solution. I do wonder how Phillip managed to
>>>> miss it, though, since the test script complains loudly.
>>>
>>> I am unable to reproduce any linting errors when running this script
>>> through chainlint, which is why I was more than a little confused by
>>> this patch when I read it, and I was just about to ask for more
>>> information, such as the actual error message.
>>
>> It's not your chain-lint script, but rather the builtin one that sticks
>> "(exit 117) &&" in front of the snippet and evals it. So it creates the
>> exact "foo && bar &" situation by prepending a line to the snippet.
> 
> And btw, I think that is the answer to "how did Phillip not notice it?".
> When running "make test" these days, we rely on chainlint.pl to detect
> any problems, and then set GIT_TEST_CHAIN_LINT=0 so that the scripts do
> not invoke it again. But that variable also suppresses the internal
> linter, and thus "make test" passes, but running the script individually
> does not.

Ah, that explains it, I was wondering how the CI run had passed. Thanks to Michael for the patch and Peff and Eric for digging into cause of the problem

Best Wishes
Phillip
Show 16 quoted lines
> It does seem like a recipe for confusion if the two linters are not in
> agreement. I think we might want to either:
> 
>    1. Say that the internal linter still has value, and tweak the
>       suppression so it only turns off the extra per-script run of
>       chainlint.pl, and not the internal one (which is cheap-ish to run).
> 
>    2. Say that the internal linter does not have value, and we should
>       rely on chainlint.pl. In which case we might as well ditch the
>       internal one completely.
> 
>       I'm OK with this direction, if we're comfortable that there are no
>       real problems that would be caught by the internal one but not the
>       script.
> 
> -Peff
Previous: Eric SunshineNext: Michael J Gruber
Message 15 of 22 in “wildmatch: fix exponential behavior”
  1. 0/3 wildmatch: fix exponential behaviorPhillip Wood, Mar 20, 2023
  2. 2/3 wildmatch: avoid undefined behaviorPhillip Wood, Mar 20, 2023
  3. 1/3 wildmatch: fix exponential behaviorPhillip Wood, Mar 20, 2023
  4. t3070: make chain lint tester happyMichael J Gruber, Mar 24, 2023
  5. Jeff KingMar 25, 2023
  6. Eric SunshineMar 25, 2023
  7. Jeff KingMar 25, 2023
  8. Jeff KingMar 25, 2023
  9. Eric SunshineMar 25, 2023
  10. Jeff KingMar 25, 2023
  11. Jeff KingMar 25, 2023
  12. Eric SunshineMar 25, 2023
  13. Jeff KingMar 25, 2023
  14. Eric SunshineMar 25, 2023
  15. Phillip WoodMar 26, 2023
  16. Michael J GruberMar 26, 2023
  17. Eric SunshineMar 25, 2023
  18. Jeff KingMar 25, 2023
  19. 3/3 wildmatch: hide internal return valuesPhillip Wood, Mar 20, 2023
  20. Junio C HamanoMar 20, 2023
  21. Derrick StoleeMar 23, 2023
  22. Phillip WoodMar 24, 2023

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.