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.

Sign up for GitHub

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

jj (Jujutsu) 0.4.0 (new formula) #105986

Closed
wants to merge 7 commits into from
Closed

jj (Jujutsu) 0.4.0 (new formula) #105986

Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
063fe50
jj (Jujutsu) 0.4.0 (new formula)
ClashTheBunny Jul 16, 2022
fef5f25
fixups from CI and naming convention
ClashTheBunny Jul 17, 2022
33706aa
Spell out DVCS
ClashTheBunny Jul 17, 2022
8a1bde2
Remove spurious line
ClashTheBunny Aug 22, 2022
dda9e41
Use macos openssl, don't use vendored openssl
ClashTheBunny Aug 23, 2022
854c065
Re-depend on openssl everywhere due to un-vendoring
ClashTheBunny Aug 23, 2022
906dd6f
Depend on gcc on linux
ClashTheBunny Aug 23, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
jj (Jujutsu) 0.4.0 (new formula) 
This adds the latest release as well as the ability to install HEAD.  I
used the same structure as Starship, so it should work in all the same
places. I've tested it on Monterey and Debian.
  • Loading branch information
@ClashTheBunny
ClashTheBunny committed Aug 22, 2022
commit 063fe5088256748b537dd4aa0dc39d1b3217b79d
24 changes: 24 additions & 0 deletions Formula/jj.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
class Jj < Formula
desc "Jujutsu is a Git-compatible DVCS"
homepage "https://github.com/martinvonz/jj"
url "https://github.com/martinvonz/jj/archive/refs/tags/v0.4.0.tar.gz"
sha256 "f97832d69b4e486997b9548c41f6cc945c68b8a8b546172ca92b7eb23ec71be5"
license "Apache-2.0"
head "https://github.com/martinvonz/jj.git", branch: "main"

depends_on "rust" => :build
depends_on "openssl@1.1"

on_linux do
depends_on "pkg-config" => :build
end
def install
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
depends_on "openssl@1.1"
on_linux do
depends_on "pkg-config" => :build
end
def install
depends_on "openssl@1.1"
uses_from_macos "zlib"
on_linux do
depends_on "pkg-config" => :build
end
def install

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, is the OpenSSL dependency needed on macOS? It seems like it might be Linux-only.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the directions for compiling from source, it does suggest installing openssl and adding it to the PKG_CONFIG_PATH. https://github.com/martinvonz/jj#mac

I assume the zlib just uses_from_macos is just being explicit? Just curious why it worked for me without it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's currently no linkage with OpenSSL, though, given the brew linkage output:

==>brew linkage --cached jj
System libraries:
  /usr/lib/libSystem.B.dylib
  /usr/lib/libiconv.2.dylib
  /usr/lib/libresolv.9.dylib
  /usr/lib/libz.1.dylib

Are you sure cargo makes use of OpenSSL?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume the zlib just uses_from_macos is just being explicit? Just curious why it worked for me without it.

uses_from_macos is for Linux; it does nothing on macOS.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@martinvonz, do you remember when openssl is required for macOS? I assume it could be an older version of macOS or something?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't remember if or when it's required for macOS. I added the vendoring of OpenSSL in cac93e27939362fc4a834f5e3654a5bf2eaaf618. That was for musl support on Linux. You can disable the vendored-openssl feature if you want to link against the installed OpenSSL instead on macOS.

Regarding the installation instructions, I think I just forgot to remove the suggestion to install openssl when I made vendored-openssl the default.

(Sorry about the slow response; I'm on vacation and will be for another two weeks.)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can probably keep OpenSSL as Linux-only, then. We should also disable the vendored-openssl feature, as we want to be able to ship security updates.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, so disabling vendored-openssl results in it linking with the brew openssl (uses_from_macos openssl):

randall@randalls-mbp /opt/homebrew/Library/Taps/homebrew/homebrew-core (jujutsu_new_formula)$ brew linkage --cached jj
System libraries:
  /System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation
  /System/Library/Frameworks/Security.framework/Versions/A/Security
  /System/Library/Frameworks/SystemConfiguration.framework/Versions/A/SystemConfiguration
  /usr/lib/libSystem.B.dylib
  /usr/lib/libiconv.2.dylib
  /usr/lib/libresolv.9.dylib
  /usr/lib/libz.1.dylib
Homebrew libraries:
  /opt/homebrew/opt/openssl@1.1/lib/libcrypto.1.1.dylib (openssl@1.1)
  /opt/homebrew/opt/openssl@1.1/lib/libssl.1.1.dylib (openssl@1.1)
Undeclared dependencies with linkage:
  openssl@1.1

So I've kept it as depends_on openssl with the vendored-openssl removed:

randall@randalls-mbp /opt/homebrew/Library/Taps/homebrew/homebrew-core (jujutsu_new_formula●)$ brew linkage jj 
System libraries:
  /System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation
  /System/Library/Frameworks/Security.framework/Versions/A/Security
  /System/Library/Frameworks/SystemConfiguration.framework/Versions/A/SystemConfiguration
  /usr/lib/libSystem.B.dylib
  /usr/lib/libiconv.2.dylib
  /usr/lib/libresolv.9.dylib
  /usr/lib/libz.1.dylib
Homebrew libraries:
  /opt/homebrew/opt/openssl@1.1/lib/libcrypto.1.1.dylib (openssl@1.1)
  /opt/homebrew/opt/openssl@1.1/lib/libssl.1.1.dylib (openssl@1.1)

The linux linkage reports a lint with the gcc linkage, so I added another depends_on and we're good on both platforms.

❯ brew linkage jj 
System libraries:
  /lib/x86_64-linux-gnu/libc.so.6
  /lib/x86_64-linux-gnu/libdl.so.2
  /lib/x86_64-linux-gnu/libm.so.6
  /lib/x86_64-linux-gnu/libpthread.so.0
  /lib64/ld-linux-x86-64.so.2
Homebrew libraries:
  /home/randall/.linuxbrew/lib/gcc/11/libgcc_s.so.1 (gcc)
  /home/randall/.linuxbrew/opt/openssl@1.1/lib/libcrypto.so.1.1 (openssl@1.1)
  /home/randall/.linuxbrew/opt/openssl@1.1/lib/libssl.so.1.1 (openssl@1.1)
  /home/randall/.linuxbrew/opt/zlib/lib/libz.so.1 (zlib)

system "cargo", "install", *std_cargo_args
end

test do
system bin/"jj", "init", "--git"

assert_predicate testpath/".jj", :exist?
end
end