On Tue, Mar 28, 2023 at 3:39 PM Emily Shaffer <nasamuffin@xxxxxxxxxx> wrote:
> I'll reply to this mail tomorrow-ish with the topic we'll discuss
> during the meeting, and I'll reply again afterwards with the notes.
The notes follow:
* (asynchronous) What's cooking in libification?
* Patches we sent regarding libification
* Glen: Clean up config parsing:
https://lore.kernel.org/git/pull.1463.v3.git.git.1680025914.gitgitgadget@xxxxxxxxx
* Patches for review related to the libification effort
* Ævar: Remove some implicit the\_repository:
https://lore.kernel.org/git/cover-v2-00.17-00000000000-20230328T110946Z-avarab@xxxxxxxxx
* (asynchronous) What happened in the past 1-2 weeks that interested
parties or intermittent contributors need to know?
* (asynchronous) Where are you stuck? What eng discussions do we
want to have? (We'll choose a topic from this list a day ahead of the
meeting.
* Glen: I want to change the signature of config\_fn\_t, but I’m
not sure how ML will respond to such large scale changes. I might just
try it and send it anyway, but it will take time.
* Calvin: I moved some functions from strbuf.c to abspath.c, but
then I noticed that functionally, abspath.c and path.c have
non-distinct boundaries between what they both do. I’m wondering if it
makes sense to combine both files. Also the functions defined in
abspath.c have their declarations in cache.h whereas path.c has its
functions defined in path.h. I think we should at least add abspath.h
with it included in cache.h to start reducing the size of cache.h.
* Intros (if needed)
* Emily: TL of git-core at google
* Taylor: lots of Git stuff for some time
* Elijah: at Palantir, looking for more time to work on Git
stuff but for now it's \*mostly\* free time
* Glen: git-core at google
* Calvin: git-core at google
* Daniel: TPM for git-core at google
* Siddharth: working at git-core team for just this quarter at 80% time
* Session topic: Calvin's topic - boundaries between abspath and
path and whether to combine
* Elijah: I did a series taking lots of these declarations out
of cache.h and into abspath.h and path.h
* Agree that the boundary is fuzzy, didn't try to fix it. Just
mechanically moved based on whether the impl was in abspath.c or
path.c. Might be useful for you to build on top of this series
* Calvin: extra context: cache.h is a grab bag of functions
implemented in other various .c files.
* Especially interested in discussing the boundary between these
two. Should we merge them together since they're all path-related?
* Within abspath.c we have stuff that's explicitly for absolute
or real paths. But in path.c we tend to have both.
* Log diving didn't make it clear why we made a distinction into abspath.c
* Elijah: I was trying to guess whether anybody figured out the
rationale for the split. My guess was that it was actually pretty much
random 🙂
* Elijah: Do either of them bring extra headers/reduce dep size
compared to the other one?
* Glen: Python path libraries are split into path objects and
os.Path which handles OS-specific stuff like resolving symlinks etc.
Is that a more reasonable delineation between these two things? Would
create more churn but ignoring that for the moment
* Calvin: can look into it
* Emily: does that delineation make sense when we don't have a path type?
* Glen: You could consider strbuf + path-specific functions are
more like a "path class"... that might be too small though
* Emily: are callers always using both? Are callers of abspath
ones that need a minimal set of dependencies?
* Calvin: Does Elijah's patch manage to entirely remove cache.h
from path.c?
* Elijah: would have to check
* Yes, i did manage to drop it from path.c eventually but I
also had to remove setup.c stuff into setup.h so path.c could include
setup.h instead
* Glen: Check - do we know what "success" means for this discussion?
* Calvin: What problems might we have, what questions need to be
answered about those two things? Doing cleanups like this in the
future, what does that look like?
* Emily: The fact that path.c relies on setup.h makes me wonder
how much of this is super low level utility stuff vs. how much is more
semantically high level/git-specific. That might be a place to split
too
*
* Elijah: Curious why Glen is trying to change the type? What's
wrong with it as it is?
* Glen: If we modify it a little bit then we don't have to keep
global state. For example, "the config line for the current value i'm
reading" requires global state because we don't have a way to send it
to the config\_fn\_t callback
* Glen: It'd be nice to change it… but we use it everywhere,
right? Is it too big to change?
* Elijah: Right but it gets rid of a global :) :) :)
* Glen: That's useful feedback actually!
* Emily: Probably should change it so we don't have to make
large scale change every time we want to add some functionality to
that callback (e.g. run-command.h or hook.h)
* Glen: Did people find this discussion useful? Especially for
people outside of Google? Was this this useful/how you envisioned this
meeting? What is better?
* Elijah: Wasn't sure how it would go, I was already in the
middle of a bunch of cleanup and was excited for the chance to send
those patches and get review.
* I think we maybe went a little long on the abspath/path stuff,
the discussion was useful but don't want to cut it off.
* Taylor: Didn't have many expectations anyways, this seems like
mostly a Google effort so I didn't have expectations. Did think it was
useful and interesting
* Glen: Next time can we talk about how to cope with cocci changes
e.g. `Ævar: Remove some implicit the\_repository:
https://lore.kernel.org/git/cover-v2-00.17-00000000000-20230328T110946Z-avarab@xxxxxxxxx`?
Both Elijah and I weren't sure how to review this - is it well-written
coccinelle? Will it slow down the cocci check?
* Taylor: Yeah it's tricky because it's already too expensive to
run during CI, so we might not even care that it's slowing down the
check?
* Glen: Would like to talk about it and see if we can establish
some norms on the ML - explicitly state whether we care if it's
slower, etc.
* Taylor: We have a bunch of pending cocci changes, it never
quite seems right to land big changes like this. Would be nice to talk
about when to land these - is the beginning of the release cycle
better? etc
*** Note - this conversation made it to the list at
https://lore.kernel.org/git/kl6l7cuycd3n.fsf@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/
***
