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

duck: fix build for Linux #83110

Closed
wants to merge 1 commit into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
132 changes: 122 additions & 10 deletions Formula/duck.rb
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
class Duck < Formula
desc "Command-line interface for Cyberduck (a multi-protocol file transfer tool)"
homepage "https://duck.sh/"
url "https://dist.duck.sh/duck-src-7.9.3.35019.tar.gz"
sha256 "2c58200b631a71846aa4d5b9bd5c19d7122c4d71a3765aed06b62cd4ffb8fc43"
url "https://dist.duck.sh/duck-src-7.10.1.35318.tar.gz"
sha256 "e268751603bce7f276b3173f9cffe6d9a76d34a07b9054937b53b0469e7e6b8e"
license "GPL-3.0-only"
head "https://svn.cyberduck.io/trunk/"

Expand All @@ -20,19 +20,131 @@ class Duck < Formula

depends_on "ant" => :build
depends_on "maven" => :build
depends_on "pkg-config" => :build
depends_on xcode: :build

depends_on arch: :x86_64
depends_on "libffi"
depends_on "openjdk"

uses_from_macos "zlib"

on_linux do
depends_on "alsa-lib"
depends_on "freetype"
depends_on "libx11"
depends_on "libxext"
depends_on "libxi"
depends_on "libxrender"
depends_on "libxtst"

ignore_missing_libraries "libjvm.so"
end

resource "jna" do
Copy link
Contributor

Choose a reason for hiding this comment

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

This will break when we update the JNA dependency upstream and this formulae will take the older version instead.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for following up on this. I have 2 thoughts about this:

  1. These resources will be made into formulae soon, which will be updated automatically with livecheck and brew bump.
  2. The only thing we're actually taking from JNA is the dylib/so, which is built with -compatibility_version 5 such that it should maintain ABI compatibility as long as the major version remains at 5. If and when a version 6 is released which breaks ABI compatibility, we'd either rebuild duck against version 6 (if it has been updated), or if it takes a while for upstream to update, create a jna@5 formula to use in the interim.

I'm not intimately familiar with the details of how JNA is implemented, but it's generally rare for native dynamic libraries to break ABI compatibility between minor versions, let alone point releases. I'm certainly open to being proven wrong on this but we'll have to see how the next update goes.

I should also add here that we are pushing harder to try to avoid downloading pre-built native binaries because doing so has both security and compatibility implications. However we're hoping to do so without huge inconvenience to upstream projects. In this particular case, no changes are needed in how the package is built upstream - we simply delete the binaries downloaded by Maven and replace them with ones that will eventually come from formulae.

url "https://github.com/java-native-access/jna/archive/refs/tags/5.8.0.tar.gz"
sha256 "97680b8ddb5c0f01e50f63d04680d0823a5cb2d9b585287094de38278d2e6625"
end

resource "JavaNativeFoundation" do
url "https://github.com/apple/openjdk/archive/refs/tags/iTunesOpenJDK-1014.0.2.12.1.tar.gz"
sha256 "e8556a73ea36c75953078dfc1bafc9960e64593bc01e733bc772d2e6b519fd4a"
end
Comment on lines +44 to +52
Copy link
Member

Choose a reason for hiding this comment

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

Can we add a comment explaining how to update these resources? This can be in a follow-up syntax-only PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

I am actually thinking we should make these resources into formulae. I am going to update this to the newest version and then I'd like to make formulae out of the resources and remove those parts of this formulae. All we'd have to do if the resources are formulae is link the necessary files into libexec after deleting the pre-packaged ones. The resources themselves both use GitHub so we can use livecheck to keep them up to date if they are formulae.

Copy link
Member

Choose a reason for hiding this comment

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

Sounds good to me. Thanks for working on this!

Copy link
Member Author

Choose a reason for hiding this comment

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

Could you approve again? I had to dismiss the previous approval from @SMillerDev when I updated the version.


def install
xcconfig = buildpath/"Overrides.xcconfig"
xcconfig.write <<~EOS
OTHER_LDFLAGS = -headerpad_max_install_names
EOS
ENV["XCODE_XCCONFIG_FILE"] = xcconfig
# Consider creating a formula for this if other formulae need the same library
resource("jna").stage do
os = "mac"
arch = "x86-64"
on_linux do
os = "Linux"
end

on_macos do
# Add linker flags for libffi because Makefile call to pkg-config doesn't seem to work properly.
inreplace "native/Makefile", "LIBS=", "LIBS=-L#{Formula["libffi"].opt_lib} -lffi"
# Force shared library to have dylib extension on macOS instead of jnilib
inreplace "native/Makefile", "LIBRARY=$(BUILD)/$(LIBPFX)jnidispatch$(JNISFX)",
"LIBRARY=$(BUILD)/$(LIBPFX)jnidispatch$(LIBSFX)"
end

# Don't include directory with JNA headers in zip archive. If we don't do this, they will be deleted
# and the zip archive has to be extracted to get them. TODO: ask upstream to provide an option to
# disable the zip file generation entirely.
inreplace "build.xml",
"<zipfileset dir=\"build/headers\" prefix=\"build-package-${os.prefix}-${jni.version}/headers\" />", ""

system "ant", "-Dbuild.os.name=#{os}", "-Dbuild.os.arch=#{arch}", "-Ddynlink.native=true", "-DCC=#{ENV.cc}",
"native-build-package"

cd "build" do
ENV.deparallelize
ENV["JAVA_HOME"] = Language::Java.java_home(Formula["openjdk"].version.major.to_s)

# Fix zip error on macOS because libjnidispatch.dylib is not in file list
on_macos { inreplace "build.sh", "libjnidispatch.so", "libjnidispatch.so libjnidispatch.dylib" }
# Fix relative path in build script, which is designed to be run out extracted zip archive
inreplace "build.sh", "cd native", "cd ../native"

system "sh", "build.sh"
buildpath.install shared_library("libjnidispatch")
end
end

resource("JavaNativeFoundation").stage do
on_macos do
cd "apple/JavaNativeFoundation" do
xcodebuild "VALID_ARCHS=x86_64", "-project", "JavaNativeFoundation.xcodeproj"
buildpath.install "build/Release/JavaNativeFoundation.framework"
end
end
end

on_macos do
xcconfig = buildpath/"Overrides.xcconfig"
xcconfig.write <<~EOS
OTHER_LDFLAGS = -headerpad_max_install_names
EOS
ENV["XCODE_XCCONFIG_FILE"] = xcconfig
end

os = "osx"
on_linux do
os = "linux"

# This changes allow maven to build the cli/linux project as an appimage instead of an RPM/DEB.
# This has been reported upstream at https://trac.cyberduck.io/ticket/11762#ticket.
# It has been added the version 8 milestone.
inreplace "cli/linux/build.xml", "value=\"rpm\"", "value=\"app-image\""
inreplace "cli/linux/build.xml", "<arg value=\"--license-file\"/>", ""
inreplace "cli/linux/build.xml", "<arg value=\"${license}\"/>", ""
inreplace "cli/linux/build.xml", "<arg value=\"--linux-deb-maintainer\"/>", ""
inreplace "cli/linux/build.xml", "<arg value=\"&lt;feedback@cyberduck.io&gt;\"/>", ""
inreplace "cli/linux/build.xml", "<arg value=\"--linux-rpm-license-type\"/>", ""
inreplace "cli/linux/build.xml", "<arg value=\"GPL\"/>", ""
end

revision = version.to_s.rpartition(".").last
system "mvn", "-DskipTests", "-Dgit.commitsCount=#{revision}",
"--projects", "cli/osx", "--also-make", "verify"
libexec.install Dir["cli/osx/target/duck.bundle/*"]
bin.install_symlink "#{libexec}/Contents/MacOS/duck" => "duck"
"--projects", "cli/#{os}", "--also-make", "verify"

libdir = libexec/"Contents/Frameworks"
bindir = libexec/"Contents/MacOS"
on_macos do
libexec.install Dir["cli/osx/target/duck.bundle/*"]
rm_rf libdir/"JavaNativeFoundation.framework"
libdir.install buildpath/"JavaNativeFoundation.framework"
end

on_linux do
libdir = libexec/"lib/app"
bindir = libexec/"bin"
libexec.install Dir["cli/linux/target/release/duck/*"]
end

rm libdir/shared_library("libjnidispatch")
libdir.install buildpath/shared_library("libjnidispatch")
bin.install_symlink "#{bindir}/duck" => "duck"
end

test do
Expand Down