Revert "Move GC to its own directory and library"

[vtjnash]

Reverts #54238 because it fails CI (broke the gcext test) as well as deleted a large number of other test configurations (all of the static analyzer checks)

[d-netto]

The gcext test failure seems a matter of fixing an include path.

Could you elaborate on "deleted a large number of other test configurations (all of the static analyzer checks)"?

In any case, if these are hard to fix I'm fine with reverting and re-landing later, but just want to understand why these were not caught by CI (or at least, why CI was green when I merged despite this failure).

[vtjnash]

it is green because you deleted the tests, which isn't something CI can catch. In general, we should avoid adding new Makefiles in subdirectories, as it leads to subtle bugs and makes correct Makefile rules impossible to write. Instead we should be listing all rules in the toplevel Makefile as a relative path.

[d-netto]

Sounds good, thanks for the clarification.

[fingolfin]

Th gcext header also must (still) be installed! It is a crucial part of the API for us.

I would suggest moving that header out of gc again at least, any other GC implementation should also implement those callbacks after all

[d-netto]

I would suggest moving that header out of gc again at least, any other GC implementation should also implement those callbacks after all

Unless I'm misunderstanding something, this seems to contradict your comment raised here: #54238 (comment).

we should avoid adding new Makefiles in subdirectories

The fact that the PR broke the analyzer and gcext test justify the reversion. But it would be a lot better if the objection about the Makefiles in subdirectories could have been raised in the PR review itself.

[vtjnash]

Yeah, I know it would have been better, but unfortunately I have had to be away from work for a lot of time, so post-merge review has been necessary for a time. I should be able to be better about that by August.

[fingolfin]

@d-netto why are you misquoting me? The comment you link to does not say what you claim it does. Rather it says:

Shouldn't src/julia_gcext.h also be moved? Basically everything it defines is implemented in src/gc.c

And I would like to point out that this was phrased as a question, quite deliberately, as it wasn't clear to me what really is going on and what the plans are. Based on what I've learned since then, it seems to me that it doesn't make sense to move this into a gc/ subdirectory: as I understand it the plan is to have gc contain just one implementation, and then have other subdirs for other GC implementations. If that is true a shared header like that shouldn't be in just one of those dirs. But perhaps I am misunderstanding?

[fingolfin]

OK, I think I misread your comment: I thought you were saying that my comment on the PR literally was we should avoid adding new Makefiles in subdirectories but I think this was a separate thing? I find this still confusing to read.

Anyway, I feel I don't understand what's the goal for this moving around. Perhaps if there was a description somewhere what the intended end state (with multiple GC implementations) is envisioned to look like and how it should work, that would help. Perhaps it exists somewhere but I didn't see it so far?

[d-netto]

Perhaps if there was a description somewhere what the intended end state (with multiple GC implementations) is envisioned to look like and how it should work, that would help. Perhaps it exists somewhere but I didn't see it so far?

Yes, this would certainly benefit from a more formal design doc/Julep. We at RAI might be working on this in July and I'm happy to share it so that it gets sufficient review from the community/other stakeholders of this part of the codebase.