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

tsduck: added dependency to zlib #204320

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

lelegard
Copy link
Contributor

The next version of tsduck will require the zlib library. This PR prepares the formula so that the autobump will not fail when the next version is released.

There is no need to regenerate the bottles for the current version of tsduck.

  • 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 HOMEBREW_NO_INSTALL_FROM_API=1 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 HOMEBREW_NO_INSTALL_FROM_API=1 brew install --build-from-source <formula>)? If this is a new formula, does it pass brew audit --new <formula>?

The next version of tsduck will require the zlib library. This PR
prepares the formula so that the autobump will not fail when the
next version is released.

There is no need to regenerate the bottles for the current version
of tsduck.
@github-actions github-actions bot added the java Java use is a significant feature of the PR or issue label Jan 15, 2025
@@ -30,6 +30,7 @@ class Tsduck < Formula
uses_from_macos "curl"
uses_from_macos "libedit"
uses_from_macos "pcsc-lite"
uses_from_macos "zlib"
Copy link
Member

Choose a reason for hiding this comment

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

If it's not required yet it should be in a head do block

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What is a head do block?

In a previous version, I already did a similar update, adding a dependency for the future version and it was accepted. In an even earlier version, a new version was autobump'ed before I had time to update the formula. This resulted in multiple build failures, discussions and retries. This is why now I anticipate this with prior update of the formula.

Copy link
Member

Choose a reason for hiding this comment

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

See

for an example of how to use a head do block to define dependencies that are not needed for a release yet.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can you explain the impact of "head do" on the next autobump?

When the bottles for the new version of the project will be built, do you confirm that the instructions in the "head do" will be used?

My understanding is that a "head" block is in use with brew install --HEAD. That's useful but not the main point of this PR. This PR was sent to avoid build failure during the autobump which will follow the next release of the project.

Copy link
Member

Choose a reason for hiding this comment

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

Can you explain the impact of "head do" on the next autobump?

Nothing will impact the next autobump because it's separate from the existing state of the formula.

When the bottles for the new version of the project will be built, do you confirm that the instructions in the "head do" will be used?

They will not, but a contributor can check the formula and see in the head block that the therefore unreleased code needed this dependency. So they can upgrade that dependency to a normal one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They will not, but a contributor can check the formula and see in the head block that the therefore unreleased code needed this dependency. So they can upgrade that dependency to a normal one.

As I explained twice in my previous posts on this thread, that is exactly what I wanted to avoid.

According to what you describe, the following scenario will take place:

  • Next version of tsduck is released on Github
  • Homebrew autobump detects it and starts to build bottles
  • The build fails (because of the missing dependency) and the PR remains in limbo.
  • The availability of the new version of tsduck in Homebrew is delayed until a maintainer realizes that there is problem.
  • The maintainer has to analyze the problem and update the formula to move this small directive uses_from_macos "zlib" from head to main.
  • The build restarts.

This is a waste of time and maintainer's effort. Having uses_from_macos "zlib" directly in the formula is harmless for the current version and required for the next one.

As I already explained, I already had similar requests twice in the past, adding dependencies to "bash" (#197934) and to "gmake" (#191200) for next versions. Both PR's were approved by two distinct Homebrew maintainers. I don't understand why you persist in this position.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
java Java use is a significant feature of the PR or issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants