Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Starting with v0.13, the official Mac (x86_64) binary no longer works on Apple Silicon Macs via Rosetta. #2896

Closed
ilyagr opened this issue Jan 30, 2024 · 6 comments

Comments

@ilyagr
Copy link
Contributor

ilyagr commented Jan 30, 2024

v0.12 official binary works fine after Rosetta 2 is installed.

v0.13 official binary results in:

$ cd jj-v0.13.0-x86_64-apple-darwin/
$ ./jj
dyld[82582]: Library not loaded: /usr/local/opt/libgit2/lib/libgit2.1.7.dylib
  Referenced from: <C7E5CA77-7147-3BB5-A67F-CAAC76589A3A> /Users/ilyagr/tmp-nobackup/jj-v0.13.0-x86_64-apple-darwin/jj
  Reason: tried: '/usr/local/opt/libgit2/lib/libgit2.1.7.dylib' (no such file), '/System/Volumes/Preboot/Cryptexes/OS/usr/local/opt/libgit2/lib/libgit2.1.7.dylib' (no such file), '/usr/local/opt/libgit2/lib/libgit2.1.7.dylib' (no such file), '/usr/local/lib/libgit2.1.7.dylib' (no such file), '/usr/lib/libgit2.1.7.dylib' (no such file, not in dyld cache)
fish: Job 1, './jj' terminated by signal SIGABRT (Abort)

Did we start linking libgit2 dynamically?

My machine is an M2 Mac, running a recent MacOS Sonoma.

$ uname -a
Darwin macaw.local 23.2.0 Darwin Kernel Version 23.2.0: Wed Nov 15 21:55:06 PST 2023; root:xnu-10002.61.3~2/RELEASE_ARM64_T6020 arm64 arm Darwin

This will be less annoying after the next release if #2895 gets merged, but it's still a little annoying.

@ilyagr
Copy link
Contributor Author

ilyagr commented Jan 30, 2024

I'm not sure what's going on. I may have missed something, but I couldn't immediately find any relevant changes in Cargo.toml or release.yml. Perhaps something changed with the "git2" library's default features? We did upgrade from git2 v0.17.2 to v0.18.1 between jj v0.12 and v0.13.

@thoughtpolice
Copy link
Member

What does otool -L on the jj binary say? I suspect if it's been broken on Rosetta, it's probably just broken on ordinary x86 machines as well, right? I don't have one to test, of course...

@thoughtpolice
Copy link
Member

What does otool -L on the jj binary say? I suspect if it's been broken on Rosetta, it's probably just broken on ordinary x86 machines as well, right? I don't have one to test, of course...

Actually, I'm not sure that's true. Otherwise, you'd think it would fail mysteriously during GitHub Actions, right? After all I wouldn't expect /usr/local/opt/libgit2/lib/libgit2.1.7.dylib to exist on a runner without us installing it somehow, but...

@ilyagr
Copy link
Contributor Author

ilyagr commented Jan 30, 2024

What does otool -L on the jj binary say?

# Working version
$ otool -L jj-v0.12.0-x86_64-apple-darwin/jj
jj-v0.12.0-x86_64-apple-darwin/jj:
        /System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation (compatibility version 150.0.0, current version 1856.105.0)
        /System/Library/Frameworks/SystemConfiguration.framework/Versions/A/SystemConfiguration (compatibility version 1.0.0, current version 1163.60.3)
        /usr/lib/libiconv.2.dylib (compatibility version 7.0.0, current version 7.0.0)
        /System/Library/Frameworks/Security.framework/Versions/A/Security (compatibility version 1.0.0, current version 60157.60.19)
        /usr/lib/libz.1.dylib (compatibility version 1.0.0, current version 1.2.11)
        /usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1311.0.0)

# Non-working version
$ otool -L jj-v0.13.0-x86_64-apple-darwin/jj
jj-v0.13.0-x86_64-apple-darwin/jj:
        /System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation (compatibility version 150.0.0, current version 1856.105.0)
        /System/Library/Frameworks/SystemConfiguration.framework/Versions/A/SystemConfiguration (compatibility version 1.0.0, current version 1163.60.3)
        /usr/local/opt/libgit2/lib/libgit2.1.7.dylib (compatibility version 1.7.0, current version 1.7.1)
        /usr/lib/libz.1.dylib (compatibility version 1.0.0, current version 1.2.11)
        /usr/lib/libiconv.2.dylib (compatibility version 7.0.0, current version 7.0.0)
        /usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1311.0.0)

Diff:

--- 0.12-otool  2024-01-30 14:54:36
+++ 0.13-otool  2024-01-30 14:54:29
@@ -1,7 +1,7 @@
-jj-v0.12.0-x86_64-apple-darwin/jj:
+jj-v0.13.0-x86_64-apple-darwin/jj:
        /System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation (compatibility version 150.0.0, current version 1856.105.0)
        /System/Library/Frameworks/SystemConfiguration.framework/Versions/A/SystemConfiguration (compatibility version 1.0.0, current version 1163.60.3)
-       /usr/lib/libiconv.2.dylib (compatibility version 7.0.0, current version 7.0.0)
-       /System/Library/Frameworks/Security.framework/Versions/A/Security (compatibility version 1.0.0, current version 60157.60.19)
+       /usr/local/opt/libgit2/lib/libgit2.1.7.dylib (compatibility version 1.7.0, current version 1.7.1)
        /usr/lib/libz.1.dylib (compatibility version 1.0.0, current version 1.2.11)
+       /usr/lib/libiconv.2.dylib (compatibility version 7.0.0, current version 7.0.0)
        /usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1311.0.0)

(I haven't thought about it much, though)

Actually, I'm not sure that's true. Otherwise, you'd think it would fail mysteriously during GitHub Actions, right? After all I wouldn't expect /usr/local/opt/libgit2/lib/libgit2.1.7.dylib to exist on a runner without us installing it somehow, but...

It seems we're confused to more or less the same degree. :) I have no idea how and why tests seem to pass on x86 Mac-s.

My current guess is that "git2" changed something.

ilyagr added a commit to ilyagr/jj that referenced this issue Feb 3, 2024
This mostly reverts jj-vcs#2901 as well as its
fixup jj-vcs#2903. The related bug is reopened,
see jj-vcs#2869 (comment).

The problem is that while the fix did fix jj-vcs#2896 in most cases, it did
reintroduce the more severe bug jj-vcs#2760
in one case, if the working copy is the commit being rebased.

For example, suppose you have the tree

```
root -> A -> B -> @ (empty) -> C
```

### Before this commit

#### Case 1

`jj rebase -s B -d root` would work perfectly before this commit, resulting in

```
root -> A
  \-------B -> C
           \- @ (new, empty)
```

#### Case 2

Unfortunately, if you run `jj rebase -s @ -d A`, you'd have the following result
(before this commit), which shows the reintroduction of jj-vcs#2760:

```
root -> A @ -> C
         \-- B
```

with the working copy at `A`. The reason for this is explained in
jj-vcs#2901 (comment).

### After this commit

After this commit, both case 1 and case 2 will be wrong in the sense of jj-vcs#2896,
it will not exhibit the worse bug jj-vcs#2760.

Case 1 would result in:

```
root -> A
  \-------B -> @ (empty) -> C
```

Case 2 would result in:

```
root -> A -> @ -> C
         \-- B
```

with the working copy remaining a descendant of A
ilyagr added a commit to ilyagr/jj that referenced this issue Feb 3, 2024
This mostly reverts jj-vcs#2901 as well as its
fixup jj-vcs#2903. The related bug is reopened,
see jj-vcs#2869 (comment).

The problem is that while the fix did fix jj-vcs#2896 in most cases, it did
reintroduce the more severe bug jj-vcs#2760
in one case, if the working copy is the commit being rebased.

For example, suppose you have the tree

```
root -> A -> B -> @ (empty) -> C
```

### Before this commit

#### Case 1

`jj rebase -s B -d root --abandon-empty` would work perfectly before this
commit, resulting in

```
root -> A
  \-------B -> C
           \- @ (new, empty)
```

#### Case 2

Unfortunately, if you run `jj rebase -s @ -d A --abandon-empty`, you'd have the
following result (before this commit), which shows the reintroduction of jj-vcs#2760:

```
root -> A @ -> C
         \-- B
```

with the working copy at `A`. The reason for this is explained in
jj-vcs#2901 (comment).

### After this commit

After this commit, both case 1 and case 2 will be wrong in the sense of jj-vcs#2896,
but it will no longer exhibit the worse bug jj-vcs#2760 in the second case.

Case 1 would result in:

```
root -> A
  \-------B -> @ (empty) -> C
```

Case 2 would result in:

```
root -> A -> @ -> C
         \-- B
```

with the working copy remaining a descendant of A
ilyagr added a commit to ilyagr/jj that referenced this issue Feb 3, 2024
This mostly reverts jj-vcs#2901 as well as its
fixup jj-vcs#2903. The related bug is reopened,
see jj-vcs#2869 (comment).

The problem is that while the fix did fix jj-vcs#2896 in most cases, it did
reintroduce the more severe bug jj-vcs#2760
in one case, if the working copy is the commit being rebased.

For example, suppose you have the tree

```
root -> A -> B -> @ (empty) -> C
```

### Before this commit

#### Case 1

`jj rebase -s B -d root --abandon-empty` would work perfectly before this
commit, resulting in

```
root -> A
  \-------B -> C
           \- @ (new, empty)
```

#### Case 2

Unfortunately, if you run `jj rebase -s @ -d A --abandon-empty`, you'd have the
following result (before this commit), which shows the reintroduction of jj-vcs#2760:

```
root -> A @ -> C
         \-- B
```

with the working copy at `A`. The reason for this is explained in
jj-vcs#2901 (comment).

### After this commit

After this commit, both case 1 and case 2 will be wrong in the sense of jj-vcs#2896,
but it will no longer exhibit the worse bug jj-vcs#2760 in the second case.

Case 1 would result in:

```
root -> A
  \-------B -> @ (empty) -> C
```

Case 2 would result in:

```
root -> A -> @ -> C
         \-- B
```

with the working copy remaining a descendant of A
@thoughtpolice
Copy link
Member

The official 0.14 macOS binaries have mysteriously and magically fixed themselves:

aseipp@navi tmp % otool -L jj-0.13
jj-0.13:
	/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation (compatibility version 150.0.0, current version 1856.105.0)
	/System/Library/Frameworks/SystemConfiguration.framework/Versions/A/SystemConfiguration (compatibility version 1.0.0, current version 1163.60.3)
	/usr/local/opt/libgit2/lib/libgit2.1.7.dylib (compatibility version 1.7.0, current version 1.7.1)
	/usr/lib/libz.1.dylib (compatibility version 1.0.0, current version 1.2.11)
	/usr/lib/libiconv.2.dylib (compatibility version 7.0.0, current version 7.0.0)
	/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1311.0.0)
aseipp@navi tmp % otool -L jj-0.14-x86_64
jj-0.14-x86_64:
	/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation (compatibility version 150.0.0, current version 1856.105.0)
	/System/Library/Frameworks/SystemConfiguration.framework/Versions/A/SystemConfiguration (compatibility version 1.0.0, current version 1163.60.3)
	/usr/lib/libiconv.2.dylib (compatibility version 7.0.0, current version 7.0.0)
	/System/Library/Frameworks/Security.framework/Versions/A/Security (compatibility version 1.0.0, current version 60157.60.19)
	/usr/lib/libz.1.dylib (compatibility version 1.0.0, current version 1.2.11)
	/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1311.0.0)

So maybe we can close this? We could also mark the 0.13 GitHub release to retroactively note that the 0.13 build is broken on macOS.

@ilyagr
Copy link
Contributor Author

ilyagr commented May 20, 2024

Indeed, the jj v0.17.1 for x86 magically works for me. Great! I guess we'll never know what happened.

@ilyagr ilyagr closed this as completed May 20, 2024
ilyagr added a commit to ilyagr/jj that referenced this issue Jul 26, 2024
…link libgit2

This changes less than it seems. Our CI builds already mostly linked a vendored
copy of libgit2. This is because before this commit, it turns out that `git2`
could link `libgit2` *either* statically or dynamically based on whether it could
find a version of libgit2 it liked to link dynamically. Our CI builds usually did
not provide such a version AFAIK.

This made the kind of binary `cargo install` would produce unpredictable and may
have contributed to jj-vcs#2896.

Instead, if a packager wants to link `libgit2` dynamically, they should set an
environment variable, as described inside the diff of this commit. I also think
we should recommend static linking as `git2` is quite picky about the versions of
`libgit2` it supports. See also rust-lang/git2-rs#1073

This might be related to jj-vcs#4115.
ilyagr added a commit to ilyagr/jj that referenced this issue Jul 26, 2024
…link libgit2

This changes less than it seems. Our CI builds already mostly linked a vendored
copy of libgit2. This is because before this commit, it turns out that `git2`
could link `libgit2` *either* statically or dynamically based on whether it could
find a version of libgit2 it liked to link dynamically. Our CI builds usually did
not provide such a version AFAIK.

This made the kind of binary `cargo install` would produce unpredictable and may
have contributed to jj-vcs#2896.

Instead, if a packager wants to link `libgit2` dynamically, they should set an
environment variable, as described inside the diff of this commit. I also think
we should recommend static linking as `git2` is quite picky about the versions of
`libgit2` it supports. See also rust-lang/git2-rs#1073

This might be related to jj-vcs#4115.
ilyagr added a commit to ilyagr/jj that referenced this issue Jul 26, 2024
…link libgit2

This changes less than it seems. Our CI builds already mostly linked a vendored
copy of libgit2. This is because before this commit, it turns out that `git2`
could link `libgit2` *either* statically or dynamically based on whether it could
find a version of libgit2 it liked to link dynamically. Our CI builds usually did
not provide such a version AFAIK.

This made the kind of binary `cargo install` would produce unpredictable and may
have contributed to jj-vcs#2896.  I was once very surprised when I did `brew upgrade libgit2` and then
`cargo build --release` suddenly switched from building dynamically linked `jj` to the vendored version.

Instead, if a packager wants to link `libgit2` dynamically, they should set an
environment variable, as described inside the diff of this commit. I also think
we should recommend static linking as `git2` is quite picky about the versions of
`libgit2` it supports. See also rust-lang/git2-rs#1073

This might be related to jj-vcs#4115.
ilyagr added a commit to ilyagr/jj that referenced this issue Jul 28, 2024
…link libgit2

This changes less than it seems. Our CI builds already mostly linked a vendored
copy of libgit2. This is because before this commit, it turns out that `git2`
could link `libgit2` *either* statically or dynamically based on whether it could
find a version of libgit2 it liked to link dynamically. Our CI builds usually did
not provide such a version AFAIK.

This made the kind of binary `cargo install` would produce unpredictable and may
have contributed to jj-vcs#2896.  I was once very surprised when I did `brew upgrade libgit2` and then
`cargo build --release` suddenly switched from building dynamically linked `jj` to the vendored version.

Instead, if a packager wants to link `libgit2` dynamically, they should set an
environment variable, as described inside the diff of this commit. I also think
we should recommend static linking as `git2` is quite picky about the versions of
`libgit2` it supports. See also rust-lang/git2-rs#1073

This might be related to jj-vcs#4115.
ilyagr added a commit that referenced this issue Jul 28, 2024
…link libgit2

This changes less than it seems. Our CI builds already mostly linked a vendored
copy of libgit2. This is because before this commit, it turns out that `git2`
could link `libgit2` *either* statically or dynamically based on whether it could
find a version of libgit2 it liked to link dynamically. Our CI builds usually did
not provide such a version AFAIK.

This made the kind of binary `cargo install` would produce unpredictable and may
have contributed to #2896.  I was once very surprised when I did `brew upgrade libgit2` and then
`cargo build --release` suddenly switched from building dynamically linked `jj` to the vendored version.

Instead, if a packager wants to link `libgit2` dynamically, they should set an
environment variable, as described inside the diff of this commit. I also think
we should recommend static linking as `git2` is quite picky about the versions of
`libgit2` it supports. See also rust-lang/git2-rs#1073

This might be related to #4115.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants