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

protobuf 3.19.1 #85282

Closed
wants to merge 29 commits into from
Closed

Conversation

moonfruit
Copy link
Contributor

Created with brew bump-formula-pr.

@BrewTestBot BrewTestBot added automerge-skip `brew pr-automerge` will skip this pull request bump-formula-pr PR was created using `brew bump-formula-pr` labels Sep 16, 2021
@carlocab carlocab added CI-long-timeout Use longer GitHub Actions CI timeout. CI-no-fail-fast Continue CI tests despite failing GitHub Actions matrix builds. labels Sep 16, 2021
@github-actions
Copy link
Contributor

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. To keep this pull request open, add a help wanted or in progress label.

@github-actions github-actions bot added the stale No recent activity label Sep 19, 2021
@carlocab carlocab added help wanted Task(s) needing PRs from the community or maintainers and removed stale No recent activity labels Sep 19, 2021
@Moisan
Copy link
Member

Moisan commented Sep 23, 2021

The removal of SetTotalBytesLimit with two arguments is one of the main build failure culprit. The upstream change for all those projects is quite simple: remove the second argument of the call, it was ignored anyway until now.

@Moisan
Copy link
Member

Moisan commented Sep 24, 2021

opencv issue has been fixed upstream and should be in the next release: opencv/opencv#20386

@nipunn1313
Copy link

The removal of SetTotalBytesLimit with two arguments is one of the main build failure culprit. The upstream change for all those projects is quite simple: remove the second argument of the call, it was ignored anyway until now.

Can we accelerate this PR by changing dependencies of broken upstream formulas from protobuf to protobuf@3.17.3 until the upstreams fix themselves (eg zbackup)? It seems unwise to wait on all these upstreams in the PR. As the individual upstreams fix themselves, they can switch back.

@SMillerDev
Copy link
Member

The best option is to make upstream pull requests, the second best is upstream issues. If upstream says they won't fix it we can always switch to an older protobuf, but that should be a last resort.

@nipunn1313
Copy link

Making PRs to update the upstreams makes sense to me - but it also seems like having a tree of blockers on other projects like this will cause delays in making protobuf 3.18.0 available to folks (eg it's already been 2 weeks). To me - these delays seem unnecessary. For example - we can invert the sequencing and release protobuf 3.18.0 and fix the upstreams w/ PRs to follow.

I'm here to learn - so please let me know if there's some hole in my reasoning or something I'm not considering that's important here.

@SMillerDev
Copy link
Member

History has taught us that if we point projects to older versions of dependencies, we are then stuck with the older version forever. I don't think I've ever seen a PR by someone who went to upstream to fix compatibility with newer versions if they weren't somehow forced to.

For reference you can read the discussions in the PRs about Go 1.17, Go 1.16, Go 1.15 and probably every Go version since Homebrew started packaging Go.

@Moisan Moisan mentioned this pull request Oct 2, 2021
@Moisan
Copy link
Member

Moisan commented Oct 2, 2021

shogun doesn't currently build on master and the issue has been known for more than a year upstream: shogun-toolbox/shogun#4870

@nipunn1313
Copy link

For zbackup - there's a PR here to fix zbackup/zbackup#158 and zbackup/zbackup#159. It appears zbackup doesn't build on mac off of master (/Users/nipunn/src/zbackup/buse.c:23:10: fatal error: 'linux/types.h' file not found ), but does build off the stable branch. Also I noted that there haven't been commits in 5 years.

@moonfruit moonfruit changed the title protobuf 3.18.0 protobuf 3.18.1 Oct 8, 2021
@iMichka iMichka mentioned this pull request Oct 21, 2021
6 tasks
@thibault-ml
Copy link

History has taught us that if we point projects to older versions of dependencies, we are then stuck with the older version forever.

That's understandable, but for things like zbackup which have not been updated in 5 years, I would consider that isn't too much of a problem.
If we don't do it for zbackup (at least), we risk not being able to update protobuf for years to come, which I hope we can all agree is not acceptable.

Is there a threshold of "out-of-date-ness" whereby a dependency is considered "abandoned" and shouldn't be relied upon to be updated?

@nipunn1313
Copy link

Looks like zbackup is being deprecated #87727
In my experience, it's a challenging (and losing) game to block on (implicitly taking ownership of) other projects' dependencies.

It's a delicate balance. It's great to have dependency updates happen quickly on release, but you really don't want a priority inversion where an update to an important project (eg protobuf) is blocked on an update to a less important project (eg zbackup).

Need both a carrot and a stick.

The carrot (incentive) of "upgrade dependencies ASAP - it's blocking us from adopting" is really nice for providing a community incentive for timely dependency upgrade - especially for the trivial fixes. Means homebrew is less likely to be holding onto tons of old versions.

The corresponding stick would be - "we're leaving you behind if you don't update". Eg. if X weeks goes by after a release and a dependent project is dragging its feet, then we gotta drop it from homebrew (or pin to old version and hang onto the old version).

I feel like there's a potential for a best of both worlds solution. Straw man of solution (using this case as example)

  • New protobuf comes out (3.18). It gets added immediately to homebrew, while old version (3.17) sticks around - incompatible projects are pinned to old versions. 3.17 is marked for deprecation in X weeks
  • Build a tool that lists off projects that are affected by the deprecation (eg zbackup, opencv, libtorch). They can be sorted, prioritized, and fixed appropriately (filing tasks against affected projects).
  • After X weeks, deprecated versions are eliminated from homebrew (3.17).
  • Also after X weeks, remaining affected projects that don't build (eg zbackup) are deprecated/removed (the stick).

This strategy carries the risk of getting soft with "X". Picking some value of X (eg 4 weeks) and being serious about it takes diligence. Have to really enforce the stick and deprecate things that are dragging their feet w/ dependency changes. Good tooling and up front honesty about timelines help. X would probably have to be bigger for more major changes (eg new version of go).

@SMillerDev
Copy link
Member

That would be great if we had the amount of people that it takes to do that, or people actually got paid to do it, until then we're going to stick with the approach that works for homebrew.

@SMillerDev
Copy link
Member

Additionally, instead of coming up with new patch management schemes for homebrew it would be more beneficial to send patches to the blocking upstream projects or deprecate their homebrew Formula.

@nipunn1313
Copy link

That would be great if we had the amount of people that it takes to do that, or people actually got paid to do it, until then we're going to stick with the approach that works for homebrew.

Yeah - that makes sense. There's definitely additional maintenance to something like that.

@thibault-ml
Copy link

That would be great if we had the amount of people that it takes to do that, or people actually got paid to do it, until then we're going to stick with the approach that works for homebrew.

Totally agree and understand that.

What is the current policy or procedure that works for homebrew? Is it made explicit somewhere?

@SMillerDev
Copy link
Member

What is the current policy or procedure that works for homebrew? Is it made explicit somewhere?

We upgrade everything as soon as we can.

No special cases, no caveats. If something doesn't build we work with the community to fix it. If we can't fix it we disable/deprecate it.

@moonfruit moonfruit changed the title protobuf 3.18.1 protobuf 3.19/0 Oct 26, 2021
@carlocab
Copy link
Member

Fixed the alias (it was still named protobuf@3.18) and added bear and mavsdk.

@BrewTestBot
Copy link
Member

:shipit: @iMichka has triggered a merge.

@iMichka
Copy link
Member

iMichka commented Dec 22, 2021

Done. I'll check what is going on with qt-percona-server and percona-xtrabackup later today.

@iMichka iMichka removed the CI-long-timeout Use longer GitHub Actions CI timeout. label Dec 22, 2021
@thibault-ml
Copy link

Thanks all! Appreciate the effort of all involved!

@github-actions github-actions bot added the outdated PR was locked due to age label Jan 22, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 22, 2022
@moonfruit moonfruit deleted the bump-protobuf-3.18.0 branch February 25, 2022 03:01
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
automerge-skip `brew pr-automerge` will skip this pull request bump-formula-pr PR was created using `brew bump-formula-pr` CI-linux-self-hosted Build on Linux self-hosted runner CI-no-fail-fast Continue CI tests despite failing GitHub Actions matrix builds. help wanted Task(s) needing PRs from the community or maintainers outdated PR was locked due to age
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants