Skip to content

firefoxpwa: use openssl@3, bzip2: add pkgconfig file#113581

Closed
cho-m wants to merge 2 commits intoHomebrew:masterfrom
cho-m:firefoxpwa-openssl3
Closed

firefoxpwa: use openssl@3, bzip2: add pkgconfig file#113581
cho-m wants to merge 2 commits intoHomebrew:masterfrom
cho-m:firefoxpwa-openssl3

Conversation

@cho-m
Copy link
Member

@cho-m cho-m commented Oct 20, 2022

  • 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>?

@cho-m cho-m added the openssl-3-migration Related to switching to an OpenSSL 3 dependency label Oct 20, 2022
@BrewTestBot BrewTestBot added the rust Rust use is a significant feature of the PR or issue label Oct 20, 2022
@cho-m cho-m added do not merge in progress Stale bot should stay away labels Oct 20, 2022
@cho-m cho-m force-pushed the firefoxpwa-openssl3 branch from e3e5883 to dff2458 Compare November 3, 2022 10:11
@cho-m cho-m added the CI-no-fail-fast Continue CI tests despite failing GitHub Actions matrix builds. label Nov 3, 2022
Formula/bzip2.rb Outdated
Comment on lines 41 to 56
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 not add this as a patch?

Copy link
Member Author

Choose a reason for hiding this comment

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

The 1.1.x repo is separate work from 1.0.x repo (ref: https://sourceware.org/bzip2/downloads.html) and uses different build tools (previously autotools and now cmake/meson) rather than 1.0.x Makefile.

Some Linux distros have been maintaining their own copies of the pkgconfig file, e.g.:

Was mainly testing that this worked. It looks like the rust bindings did dynamically link now (ref: https://github.com/alexcrichton/bzip2-rs/blob/bzip2-sys-0.1.10+1.0.8/bzip2-sys/build.rs#L16-L22)

@cho-m cho-m added the CI-linux-self-hosted Build on Linux self-hosted runner label Nov 3, 2022
@cho-m cho-m force-pushed the firefoxpwa-openssl3 branch from dff2458 to c1d0451 Compare November 6, 2022 20:29
@cho-m cho-m marked this pull request as ready for review November 6, 2022 20:41
@cho-m cho-m force-pushed the firefoxpwa-openssl3 branch from c1d0451 to a462b43 Compare November 30, 2022 09:22
@cho-m cho-m removed do not merge in progress Stale bot should stay away labels Dec 1, 2022
@cho-m cho-m changed the title firefoxpwa: use openssl@3 firefoxpwa: use openssl@3, bzip2: add pkgconfig file Dec 1, 2022
@cho-m
Copy link
Member Author

cho-m commented Dec 2, 2022

Linux errors are unrelated:

@chenrui333
Copy link
Member

Merged the emscripten patch PR

@p-linnane
Copy link
Member

macOS has rpath issues with gdal

Linux:

Error: 5 failed steps!
brew install gnirehtet
brew test --retry --verbose pyflow
brew test --retry --verbose rpm
brew test --retry --verbose streamlink
brew test --retry --verbose walkmod

pyflow is a known issue, API just redirects to a Heroku landing page
walkmod was deprecated in #118826.

@p-linnane p-linnane mentioned this pull request Dec 30, 2022
@cho-m cho-m force-pushed the firefoxpwa-openssl3 branch from 921aec5 to ee65754 Compare December 31, 2022 05:34
@cho-m cho-m force-pushed the firefoxpwa-openssl3 branch from ee65754 to 4fad972 Compare January 1, 2023 09:33
@cho-m
Copy link
Member Author

cho-m commented Jan 2, 2023

Looks ready to me now but would like some feedback.

Linux linkage for firefoxpwa is correct:

==> brew linkage --cached firefoxpwa
System libraries:
  /lib/x86_64-linux-gnu/libc.so.6
  /lib/x86_64-linux-gnu/libgcc_s.so.1
  /lib/x86_64-linux-gnu/libm.so.6
Homebrew libraries:
  /home/linuxbrew/.linuxbrew/opt/bzip2/lib/libbz2.so.1.0 (bzip2)
  /home/linuxbrew/.linuxbrew/opt/openssl@3/lib/libcrypto.so.3 (openssl@3)
  /home/linuxbrew/.linuxbrew/opt/openssl@3/lib/libssl.so.3 (openssl@3)

As I mentioned in file changes, these are Linux-only. Can see this via:

cargo tree --target x86_64-apple-darwin --invert bzip2-syscargo tree --target x86_64-apple-darwin --invert openssl-syscargo tree --target x86_64-unknown-linux-gnu --invert bzip2-sys
bzip2-sys v0.1.11+1.0.8
└── bzip2 v0.4.3
    └── firefoxpwa v0.0.0 (/Users/cho-m/Code/PWAsForFirefox-2.3.0/native)cargo tree --target x86_64-unknown-linux-gnu --invert openssl-sys
openssl-sys v0.9.80
├── native-tls v0.2.11
│   ├── hyper-tls v0.5.0
│   │   └── reqwest v0.11.13
│   │       └── firefoxpwa v0.0.0 (/Users/cho-m/Code/PWAsForFirefox-2.3.0/native)
│   ├── reqwest v0.11.13 (*)
│   └── tokio-native-tls v0.3.0
│       ├── hyper-tls v0.5.0 (*)
│       └── reqwest v0.11.13 (*)
└── openssl v0.10.45
    └── native-tls v0.2.11 (*)

And manually adding pkg-config file is similar to other Linux distros. I restricted this to Linux as we only build a static library on macOS and we highly prefer system copy of bzip2.

@cho-m cho-m added the ready to merge PR can be merged once CI is green label Jan 2, 2023
@BrewTestBot
Copy link
Contributor

🤖 A scheduled task has triggered a merge.

@cho-m cho-m deleted the firefoxpwa-openssl3 branch October 30, 2023 22:37
@github-actions github-actions bot added the outdated PR was locked due to age label Nov 30, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 30, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

CI-linux-self-hosted Build on Linux self-hosted runner CI-no-fail-fast Continue CI tests despite failing GitHub Actions matrix builds. openssl-3-migration Related to switching to an OpenSSL 3 dependency outdated PR was locked due to age ready to merge PR can be merged once CI is green rust Rust use is a significant feature of the PR or issue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants