1961305 - mach clang-format broken with jj workspace

Status: RESOLVED FIXED

(Firefox Build System :: General, defect, P3)

Description

[Steve Fink [:sfink] [:s:]]

It tries to fall back to git, which doesn't work if there is no .git dir (as is the case with a workspace or non-colocated checkout).

Comment 1

[Steve Fink [:sfink] [:s:]]

Attachment: Bug 1961305 - Move clang format command line building to per-repo files and implement jj version r=ahal

Comment 2

[Mike Hommey [:glandium]]

Isn't git supposed to work on non-native jj repos? Shouldn't this mean jj worktrees should have a .git file like git worktrees do?

Comment 3

[Erich Gubler [:ErichDonGubler]]

There is always a Git repository with jj repos, because Jujutsu, practically speaking, only has a Git backend. In the case of a non-colocated repository, it's a bare repository stored under .jj.

One can discover the Git repository directory (bare or not) by using jj git root, starting from jj 0.28.0.

Comment 4

[Steve Fink [:sfink] [:s:]]

Right, one approach to this would be to run jj git root, set the env var $GIT_DIR to that value, and then run the native git command. JujutsuRepository does that internally when it needs to make use of git-specific functionality.

Thus far in the jj support, I've avoided falling back to git as much as possible, because:

• that would go directly to the (possibly bare) git repo, either ignoring any relevant state in the working directory or if there is a non-colocated git checkout, possibly mixing in a different working directory
• jj and git have differing notions of "current state". git uses HEAD a lot. jj has @, but it's not the same. (I tend to map HEAD to @ if it's nonempty, else @-, but that's really not the same.)
• related to the previous, sensible defaults may be different between jj and git when an explicit commit is not specified.

These are all kind of the same thing. I guess one way to look at it is to consider the case where you have several jj workspaces sharing a single bare git repo (I expect this to be the normal case.) Running mach commands from the different workspaces should do different things. Going straight to the git directory wouldn't allow this. You could fake it some by mapping the workspace's working directory commit (workspace@) to a git commit and then making sure to pass it into any git command, but that's only one piece of state and there can be more. Even just doing something based on the directory you're running from (eg relative paths) would have to be manually handled. It's better to let jj do its thing and use git for the storage layer.

Comment 5

[Steve Fink [:sfink] [:s:]]

(In reply to Mike Hommey [:glandium] from comment #2)

Isn't git supposed to work on non-native jj repos? Shouldn't this mean jj worktrees should have a .git file like git worktrees do?

I guess my more direct answer to this question is: jj knows about git and is supposed to work on native git repos, not the other way around.

It is also important for Mozilla's usage that you can always fall back to working directly with git if some functionality is missing in jj, but falling back doesn't need to be zero-friction, just feasible and realistically doable. (For example, jj will usually have its git repo in a detached HEAD state. Which you can easily manually fix to run git commands, but until you do, using git will be weird.)

So you're right, the lack of a .git entry in a jj workspace means that git commands won't "just work" from there.

Comment 6

[Erich Gubler [:ErichDonGubler]]

:sfink:

Even just doing something based on the directory you're running from (eg relative paths) would have to be manually handled.

I'm not familiar with commit-modifying operations (as opposed to work tree modifying operations) in our mach scripts minus maybe mach try, but: would GIT_WORK_TREE in tandem with GIT_DIR help here, maybe?

Comment 7

[Steve Fink [:sfink] [:s:]]

(In reply to Erich Gubler [:ErichDonGubler] from comment #6)

:sfink:

Even just doing something based on the directory you're running from (eg relative paths) would have to be manually handled.

I'm not familiar with commit-modifying operations (as opposed to work tree modifying operations) in our mach scripts minus maybe mach try, but: would GIT_WORK_TREE in tandem with GIT_DIR help here, maybe?

Maybe? It would be interesting to play with. I doubt there's much in mach other than mach try that manipulates commits, and if we switch completely to lando then that might stop using commits too. But I was thinking more of the reading side. mach lint has --outgoing, --workdir, and --rev, all of which want to start somewhere, and they feed into whatever operation it is. Would it be better to map as much as possible to git commands? I'm skeptical, because the semantics can be a little different, and I figure that when there are differences it's safer to do what is more natural with the vcs you're using. jj supports a range of different workflows, and the mapping to git (eg the current git HEAD) is a bit arbitrary and could change.

Ok, I tried this for another bug: when running mach jstests, it checks whether it should download a recent wpt manifest. I tried running the git log command from my jj workspace while setting both GIT_DIR and GIT_WORK_TREE. But it didn't help; it uses HEAD and the jj workspace doesn't have its own HEAD or .git or anything.

To make it work, the jj workspace would need to layer on top of git worktrees, or have its own dummy .git directory with .git/HEAD updated appropriately, or something. (With worktrees, .git is a pointer to the real .git, and there you have .git/worktrees/my-worktree/HEAD.) So it looks like the GIT_WORK_TREE environment variable isn't going to handle this sort of thing, at least.

Comment 8

[Steve Fink [:sfink] [:s:]]

I'm holding off landing this until bug 1962492 lands. The tests pass with both of these applied, but I haven't tested whether that bug re-breaks anything that was previously fixed. (Once that bug has landed, anyone can feel free to push this if I haven't noticed yet.)

Comment 9

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1929372

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e37c442f6af9
Move clang format command line building to per-repo files and implement jj version r=ahal

Comment 11

[amarc]

https://hg.mozilla.org/mozilla-central/rev/e37c442f6af9