git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: Jeff King <peff@peff.net>
Cc: phillip.wood@dunelm.org.uk, Junio C Hamano <gitster@pobox.com>,
	git@vger.kernel.org, Phillip Wood <phillip.wood123@gmail.com>,
	Johannes Schindelin <johannes.schindelin@gmx.de>
Subject: Re: [PATCH] test-lib.sh: discover "git" in subdirs of "contrib/buildsystems/out"
Date: Sat, 03 Dec 2022 02:41:04 +0100	[thread overview]
Message-ID: <221203.86pmd1dyqn.gmgdl@evledraar.gmail.com> (raw)
In-Reply-To: <Y4qF3iHW2s+I0yNe@coredump.intra.peff.net>


On Fri, Dec 02 2022, Jeff King wrote:

> On Fri, Dec 02, 2022 at 05:40:34PM +0100, Ævar Arnfjörð Bjarmason wrote:
>
>> > [1] While our CI helps the MSVC job runs CMake manually, performs an
>> > in-tree build and does not use ctest. In contrast a user running the 
>> > MSVC GUI does not run CMake themselves, ends up with an out-of-tree
>> > build and runs the tests with ctest.
>> 
>> I don't run Windows by choice, and I'm not interested in running a
>> propriterary IDE (VS) either.
>> 
>> The main reason I'm working on this series is that while we as a project
>> are happy to support proprietary OS's, it hasn't been a requirement for
>> participation that you need to buy a copy of Windows, OSX, AIX, HP/UX or
>> whatever to submit patches.
>> 
>> Of course we have platform-specific code. but this CMake component is
>> unique in how invasive it is.
>> 
>> It's easy to e.g. stay away from the OSX-specific code in
>> compat/fsmonitor/*darwin*.[ch], or generally speaking the
>> Windows-specific C code.
>> 
>> But for CMake it's become a hard requirenment for many changes, even
>> though it's a contrib/ component.
>
> I have similar feelings to you here. Back when cmake support was
> introduced, I explicitly wanted it to be something for people who cared
> about it, but that wouldn't bother people who didn't use it:
>
>   https://lore.kernel.org/git/20200427200852.GC1728884@coredump.intra.peff.net/
>
> I stand by that sentiment, but it seems to have crept up as a required
> thing to deal with, and that is mostly because of CI. Using cmake in CI
> is good for telling developers when a change they make has broken cmake.
> But it also makes cmake their problem, and not the folks interested in
> cmake.

That's a bit of a pain, but I don't think the main problem is its
integration with CI. It's that there doesn't really seem to be an
interest in its active maintenance & review from its supposed main
target audience.

Case in point this "ab/cmake-nix-and-ci" topic: It's been queued for
around 2 months now.

I don't use Windows or VS. I'd just like it not to take half may day to
throw things at Windows CI whenever some obscure aspect of cmake breaks.

The only person who's been showing it any notable review interest
(Phillip) apparently hasn't used Windows actively since the mid-90's :)

Which is a very different situation than if it were still something that
broke CI, but there was someone (or more than one person) active on list
willing to test patches to get it working, was familiar with the cmake
language & could help write the cmake version of Makefile changes when
those were needed etc.

> Now maybe attitudes have changed, and I am out of date, and cmake
> support is considered mature and really important (or maybe nobody even
> agreed with me back then ;) ). But if not, should we consider softening
> the CI output so that cmake failures aren't "real" failures? That seems
> drastic and mean, and I don't like it. But it's the root of the issue,
> IMHO.

Yeah, maybe. Maybe if we broke it we'd get people showing up to maintain
it again :)

I do think the series I've got here is the most practical way forward at
this point (any outstanding issues aside). I.e. it's usually fairly
easy-ish to amend the cmake recipe.

It's just been taking *ages* because it's been a dumpste fire outside
of Windows, so if you don't have that OS available to you you need to
wait for the CI. Being able to run it in seconds on *nix really helps..

> As a side note, this isn't the only such instance of this problem. Two
> other things to think about:
>
>   - You mentioned darwin fsmonitor code. And it's true that you can
>     largely ignore it if you don't touch it. But every once in a while
>     you get bit by it (e.g., enabling a new compiler warning which
>     triggers in code you don't compile on your platform, and now you
>     have to guess-and-check the fix with CI). This sucks, but is kind of
>     inevitable on a cross-platform system. I think the issue with cmake
>     is that because it's basically duplicating/wrapping the Makefile, it
>     _feels_ unnecessary to people on platforms with working make, and
>     triggers more frequently (because changes to the rest of the build
>     system may break cmake in subtle ways).

Yeah, we'll always have some cross-platform pain.

But e.g. "chmod +x" just works in the Makefile, including when we run it
on Windows. And I've run it on Windows CI. But just upthread of here
Phillip is reporting that it doesn't work from the context of the CMake
recipe.

I've been throwing some things at Windows CI, but I'm pretty stumped as
to what that might be.

Some warning on Mac OS X is trivial by comparison :)

>   - I'd actually put the leak-checking CI in the same boat. It's a good
>     goal, and one I hope we work towards. But it feels like the current
>     state is not very mature, and people often end up wrestling with CI
>     to deal with failures that they didn't even introduce (e.g., adding
>     a new test that happens to run a Git program that has an existing
>     leak, and now you are on the hook for figuring out why the existing
>     "passes leaks" annotation is wrong).
>
>     My original hope is that we would introduce leak-checking tooling
>     that people interested in leaks could use, and other people could
>     ignore until we reached a leak-free state. Because it's in CI it
>     means that people get notified of new leaks in code they write
>     (which is good, and helps people interested in leaks), but it also
>     means they have to deal with the immature state.
>
> I'm not necessarily proposing to drop the leaks CI job here. I'm mostly
> philosophizing about the greater problem. In the early days of Git, the
> cross-platform testing philosophy was: somebody who cares will test on
> that platform and write a patch. If they don't, how important could it
> be? With CI that happens automatically and it becomes everybody's
> problem, which is a blessing and a curse.

That's a definitely a bit of an irresistible digression :)

First, I think we can agree that however frustrating that's been (and
sorry!) it would be a lot worse if my average response time to the leak
testing breaking something was measured in months :)

I do think that whatever issues we've had with it (and in retrospect I'd
do some of it differently) that it's a lot more mature these days than
you might remember.

I've been making an effort to specifically address the sorts of leaks
that would cause that sort of pain, e.g. adding an unsespecting "git
log" to an existing test file and the like.

I have a report I generate [1] of the outstanding leaks, sorted by
de-duping stack traces, and an estimate of how many tests would start
passing if a combination of leaks were solved[1].

On the one hand it's ~60k lines of scary stack traces, but on the other
hand it used to be easily 3-4x that (I can't recall the exact size
offhand).

But more importantly while we do have some widespread leaks still, it
used to be the case that e.g. some leaks would show up in 50-100 test
files.

Now the #1 leak by number of times it's seen in test files happens in
just 12 filess, and there's a very long tail of leaks that only happen
in one test somewhere. I.e. are specific to their own area.

Just anecdotally I think it's had sort of the opposite problem recently,
that it's getting good enough to start flagging about new leaks that
really are specific to newly queued topics. E.g. [2] and [3] are two
recent examples.

But in both cases there seems to have been an understandable assumption
that it was probably just "linux-leaks" being noisy, due to the job
crying wolf in the past.

1. https://vm.nix.is/~avar/noindex/2022-12-03-git-leak-report.txt
2. https://lore.kernel.org/git/221108.864jv9sc9r.gmgdl@evledraar.gmail.com/
3. https://lore.kernel.org/git/2488058d-dc59-e8c1-0611-fbcaeb083d73@web.de/

  parent reply	other threads:[~2022-12-03  2:09 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-29  9:40 What's cooking in git.git (Nov 2022, #07; Tue, 29) Junio C Hamano
2022-11-29 18:59 ` ab/remove--super-prefix and -rc0 (was What's cooking in git.git (Nov 2022, #07; Tue, 29)) Glen Choo
2022-11-30  3:43   ` Junio C Hamano
2022-11-30 18:14     ` Glen Choo
2022-11-30 19:43       ` Ævar Arnfjörð Bjarmason
2022-12-01  5:20         ` Junio C Hamano
2022-12-01 17:44         ` Glen Choo
2022-12-01 21:26           ` Junio C Hamano
2022-11-29 19:08 ` What's cooking in git.git (Nov 2022, #07; Tue, 29) Glen Choo
2022-11-30  3:45   ` Junio C Hamano
2022-11-30 18:08     ` Glen Choo
2022-11-29 21:16 ` ds/bundle-uri-4 (was Re: What's cooking in git.git (Nov 2022, #07; Tue, 29)) Derrick Stolee
2022-12-01 15:06   ` Derrick Stolee
2022-12-02  0:25     ` Junio C Hamano
2022-11-30  9:57 ` ab/cmake-nix-and-ci " Phillip Wood
2022-11-30 10:16   ` Ævar Arnfjörð Bjarmason
2022-12-01 14:23     ` Phillip Wood
2022-12-01 16:39       ` [PATCH] test-lib.sh: discover "git" in subdirs of "contrib/buildsystems/out" Ævar Arnfjörð Bjarmason
2022-12-01 16:48         ` Phillip Wood
2022-12-01 17:13           ` Ævar Arnfjörð Bjarmason
2022-12-01 23:00         ` Junio C Hamano
2022-12-02 15:14           ` Phillip Wood
2022-12-02 16:40             ` Ævar Arnfjörð Bjarmason
2022-12-02 23:10               ` Jeff King
2022-12-03  1:12                 ` Junio C Hamano
2022-12-03  1:41                 ` Ævar Arnfjörð Bjarmason [this message]
2022-12-05  9:15                   ` Jeff King
2022-12-05 23:34                   ` Taylor Blau
2022-12-05 23:46                     ` Ævar Arnfjörð Bjarmason
2022-12-06  0:35                       ` Taylor Blau
2022-12-06  1:36                     ` Jeff King
2022-12-06  1:43                       ` Ævar Arnfjörð Bjarmason
2022-12-06  2:05                         ` Jeff King
2022-12-06  2:19                           ` Ævar Arnfjörð Bjarmason
2022-12-06  3:52                             ` Junio C Hamano
2022-12-06  9:54                               ` Phillip Wood
2022-12-06 10:57                                 ` Ævar Arnfjörð Bjarmason
2022-12-08  9:29                                   ` ab/cmake-nix-and-ci, was " Johannes Schindelin
2022-12-08 11:34                                     ` Ævar Arnfjörð Bjarmason
2022-12-09  3:48                                     ` Junio C Hamano
2022-12-09 13:55                                       ` Ævar Arnfjörð Bjarmason
2022-12-07  1:00                           ` Taylor Blau
2022-11-30 10:02 ` What's cooking in git.git (Nov 2022, #07; Tue, 29) Phillip Wood

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=221203.86pmd1dyqn.gmgdl@evledraar.gmail.com \
    --to=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=johannes.schindelin@gmx.de \
    --cc=peff@peff.net \
    --cc=phillip.wood123@gmail.com \
    --cc=phillip.wood@dunelm.org.uk \
    /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).