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 7.10.1 #83146

Closed
Closed

Conversation

dkocher
Copy link
Contributor

@dkocher dkocher commented Aug 12, 2021

  • Have you followed the guidelines for contributing?
  • Have you ensured that your commits follow the commit style guide?
  • Have you checked that there aren't other open pull requests for the same formula update/change?
  • Have you built your formula locally with brew install --build-from-source <formula>, where <formula> is the name of the formula you're submitting?
  • Is your test running fine brew test <formula>, where <formula> is the name of the formula you're submitting?
  • Does your build pass brew audit --strict <formula> (after doing brew install --build-from-source <formula>)? If this is a new formula, does it pass brew audit --new <formula>?

@carlocab carlocab added the CI-no-fail-fast Continue CI tests despite failing GitHub Actions matrix builds. label Aug 12, 2021
@cho-m cho-m mentioned this pull request Aug 12, 2021
6 tasks
@danielnachun
Copy link
Member

Hi @dkocher, do you mind if I close this and combine it with the PR I have open here: #83110?. In addition to fixing the build on Linux, we will also have to fix some issues on Intel macOS with native binaries and disable building on ARM.

One question which you can answer here or in that other PR: do you know where libjnidispatch.dylib comes from in the macOS builds? This appears to be a universal binary built outside of Homebrew which normally don't allow. We do make an exception for JavaNativeFoundation.framework because it is copied from macOS itself, but we don't know if libjnidispatch.dylib also originates with macOS or is downloaded from elsewhere.

@dkocher
Copy link
Contributor Author

dkocher commented Aug 13, 2021

@danielnachun I would prefer if we can get this update through first and then handle the new builds for Linux in a separate PR. I can amend this PR to disable builds on aarch64. Can you let me know where I can find the documentation for depends_on :x86_64?

  • libjnidispatch.dylib is an upstream dependency from JNA
  • JavaNativeFoundation.framework is built from JavaNativeFoundation and is no longer distributed with macOS to my knowledge

@danielnachun
Copy link
Member

danielnachun commented Aug 13, 2021

@danielnachun I would prefer if we can get this update through first and then handle the new builds for Linux in a separate PR.

I'm totally fine with doing it this way too. I'll just rebase the other PR when this one is merged, as the only thing blocking it right are the audit issues on macOS.

I can amend this PR to disable builds on aarch64. Can you let me know where I can find the documentation for depends_on :x86_64?

I'm not sure where this is formally written up, but all that adding depends_on arch: :x86_64 will do is make sure that our CI doesn't try to build native binaries for ARM64. Instead, ARM64 users can just run the Intel Big Sur bottle on Rosetta, as they are doing with many other formulae. Let me know if that needs more clarification.

  • libjnidispatch.dylib is an upstream dependency from JNA
  • JavaNativeFoundation.framework is built from JavaNativeFoundation and is no longer distributed with macOS to my knowledge

Just to be clear, is Maven downloading those files as pre-built artifacts from somewhere? We try whenever possible to avoid doing this so that we won't have compatibility issues etc. Is there a way for us to force Maven to build those from source as well? Alternatively, we could make formulae for them and copy files/symlink into the directories, though that would be a more involved process.

@dkocher
Copy link
Contributor Author

dkocher commented Aug 13, 2021

Just to be clear, is Maven downloading those files as pre-built artifacts from somewhere? We try whenever possible to avoid doing this so that we won't have compatibility issues etc. Is there a way for us to force Maven to build those from source as well? Alternatively, we could make formulae for them and copy files/symlink into the directories, though that would be a more involved process.

All dependency management is handled by Maven and we have several an awful long list of dependencies (usually jar, but some dylib as well) that are prebuilt by us or third parties. One could certainly add Homebrew dedicated formulae but this would be an awful lot of work replicating the existing dependency management.

@dkocher
Copy link
Contributor Author

dkocher commented Aug 13, 2021

What to about the Unexpected universal binaries were found test failures?

@danielnachun
Copy link
Member

All dependency management is handled by Maven and we have several an awful long list of dependencies (usually jar, but some dylib as well) that are prebuilt by us or third parties. One could certainly add Homebrew dedicated formulae but this would be an awful lot of work replicating the existing dependency management.

I should have clarified here that we prefer to build native artifacts (MachO dylibs and executables) in CI. Java jar artifacts are totally fine to download becuase they (usually) not OS- or architecture-specific. I can see in the bottle that the other dylibs just appear to be a copy of the JRE (either from our copy or an upstream build).

What to about the Unexpected universal binaries were found test failures?

The audit failure for JavaNativeFoundation.framework can be either ignored or (if possible) have an exception, as I see it in /System/Library/Frameworks. I am thinking that for the libjnidispatch.dylib failure, we should just download https://github.com/java-native-access/jna as a resource, and use the Makefile provided by the upstream sources to build and install it, replacing the binary downloaded by Maven. This is a pretty standard practice for us - I'll test this out and see if this is a feasible approach.

@chenrui333 chenrui333 added the audit failure CI fails while auditing the software label Aug 14, 2021
@danielnachun
Copy link
Member

I ended up adding the version bump to my PR because it was simpler to do it all at once. Don't be too alarmed by all the changes in the formula - the resources blocks will be converted to formulae soon, and once we figure out how to tweak the build.xml for Linux as I mentioned over in the Cyberduck Trac, the formula will only be slightly more complex than before. I figured we'd rather get the updated version of Cyberduck to users (and make it available on Linux), even if the formula is a little complicated right now.

@github-actions github-actions bot added the outdated PR was locked due to age label Sep 16, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
audit failure CI fails while auditing the software CI-no-fail-fast Continue CI tests despite failing GitHub Actions matrix builds. outdated PR was locked due to age
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants