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.

Sign up for GitHub

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

Jump to bottom

apache-arrow and friends: use unversioned protobuf, grpc #133746

Merged
merged 12 commits into from
Jun 27, 2023
Merged

apache-arrow and friends: use unversioned protobuf, grpc #133746

merged 12 commits into from
Jun 27, 2023

Conversation

carlocab
Copy link
Member

  • 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>?
  • apache-arrow: use unversioned grpc and protobuf
  • dvc: use unversioned protobuf

@carlocab carlocab mentioned this pull request Jun 15, 2023
Closed
46 tasks
@carlocab carlocab added help wanted Task(s) needing PRs from the community or maintainers upstream issue An upstream issue report is needed labels Jun 15, 2023
@ORIOLESFan02

This comment was marked as off-topic.

Sign in to view
@scpeters
Copy link
Member

scpeters commented Jun 25, 2023 •
edited
Loading

I've worked up a patch in protobuf-c/protobuf-c#556 that lets protobuf-c compile locally on my machine, though I haven't tried running it at all (aside from brew test protobuf-c, which passes)

EDIT: built with brew install --HEAD protobuf-c with scpeters@be56ca6

@scpeters
Copy link
Member

I've worked up a patch in protobuf-c/protobuf-c#556 that lets protobuf-c compile locally on my machine, though I haven't tried running it at all (aside from brew test protobuf-c, which passes)

EDIT: built with brew install --HEAD protobuf-c with scpeters@be56ca6

@carlocab I think my changes in scpeters@be56ca6 for testing with --HEAD were a bit messy, but I'd be happy to clean it up if you have any recommendations. The patch works for me locally, but I haven't gotten any feedback on my upstream pull request

@carlocab carlocab added CI-long-timeout [DEPRECATED] Use longer GitHub Actions CI timeout. CI-no-fail-fast Continue CI tests despite failing GitHub Actions matrix builds. and removed help wanted Task(s) needing PRs from the community or maintainers labels Jun 27, 2023
@carlocab carlocab force-pushed the apache-arrow-protobuf-grpc branch from e2755b3 to aab3c61 Compare June 27, 2023 01:32
@carlocab
Copy link
Member Author

Thanks for working on that! Apologies for the delayed response; I've been finishing off #134251.

Let's give your patch a try.

@github-actions github-actions bot removed the CI-long-timeout [DEPRECATED] Use longer GitHub Actions CI timeout. label Jun 27, 2023
@carlocab carlocab force-pushed the apache-arrow-protobuf-grpc branch from aab3c61 to 2d75a6c Compare June 27, 2023 01:39
@carlocab carlocab added the CI-long-timeout [DEPRECATED] Use longer GitHub Actions CI timeout. label Jun 27, 2023
@carlocab carlocab force-pushed the apache-arrow-protobuf-grpc branch 3 times, most recently from 225e947 to 27bf919 Compare June 27, 2023 06:07
@carlocab carlocab changed the title apache-arrow: use unversioned protobuf, grpc apache-arrow, dvc, protobuf-c, etcd-cpp-apiv3: use unversioned protobuf, grpc Jun 27, 2023
@carlocab carlocab mentioned this pull request Jun 27, 2023
Closed
@carlocab carlocab force-pushed the apache-arrow-protobuf-grpc branch from 27bf919 to 3ad2300 Compare June 27, 2023 10:51
@carlocab carlocab changed the title apache-arrow, dvc, protobuf-c, etcd-cpp-apiv3: use unversioned protobuf, grpc apache-arrow and friends: use unversioned protobuf, grpc Jun 27, 2023
@carlocab carlocab added the long build Set a long timeout for formula testing label Jun 27, 2023
@carlocab carlocab mentioned this pull request Jun 27, 2023
Merged
@carlocab carlocab added the no long build conflict Do not allow merging other pull requests when files conflict with this one label Jun 27, 2023
@carlocab carlocab force-pushed the apache-arrow-protobuf-grpc branch 2 times, most recently from fcd0942 to 7ca4be7 Compare June 27, 2023 15:55
@carlocab
Copy link
Member Author

@scpeters I'm afraid we might still be blocked on netdata 🥲 The good news is that I think that should be the last one to fix.

@carlocab
Copy link
Member Author

It looks like netdata have defined a really generic macro

#define info(args...)

and this is messing with the abseil headers.

carlocab added a commit to carlocab/netdata that referenced this pull request Jun 27, 2023
@carlocab
Replace info macro with a less generic name
7508a9e 
The `info` macro is terribly generic, and can cause compilation problems
with other software.

In particular, newer versions of Protobuf (22+) pull in Abseil, which
defines an `info` function[^1] that is broken by the macro defined in
`libnetdata/log/log.h`.

This change will help unblock us at Homebrew, where we are trying to
switch our packages to using a newer version of `protobuf`.[^2]

[^1]: https://github.com/abseil/abseil-cpp/blob/e6c09ae4b2acd421a29706f86e66eaa422262ad0/absl/strings/internal/cordz_update_scope.h#L61
[^2]: Homebrew/homebrew-core#133746
carlocab added a commit to carlocab/netdata that referenced this pull request Jun 27, 2023
@carlocab
Replace info macro with a less generic name
48ca16f 
The `info` macro is terribly generic, and can cause compilation problems
with other software.

In particular, newer versions of Protobuf (22+) pull in Abseil, which
defines an `info` function[^1] that is broken by the macro defined in
`libnetdata/log/log.h`.

This change will help unblock us at Homebrew, where we are trying to
switch our packages to using a newer version of `protobuf`.[^2]

[^1]: https://github.com/abseil/abseil-cpp/blob/e6c09ae4b2acd421a29706f86e66eaa422262ad0/absl/strings/internal/cordz_update_scope.h#L61
[^2]: Homebrew/homebrew-core#133746
carlocab and others added 6 commits June 28, 2023 02:19
@carlocab
apache-arrow: use unversioned grpc and protobuf
ba3e3fd
@carlocab
dvc 3.2.2
d3f21d4 
Also, use unversioned `protobuf`.
@scpeters @carlocab
protobuf-c: switch to unversioned protobuf
6df09e1 
- add patch and HEAD
@carlocab
etcd-cpp-apiv3: use unversioned protobuf and grpc
ed522fc
@carlocab
vineyard: switch to unversioned protobuf and grpc
8d9e1ca 
This has linkage with the versioned relatives of these formulae through
`etcd-cpp-apiv3`.
@carlocab
netdata: switch to unversioned protobuf
818dd18 
This has linkage to `protobuf@21` through `protobuf-c`.
@carlocab carlocab force-pushed the apache-arrow-protobuf-grpc branch from 7ca4be7 to 818dd18 Compare June 27, 2023 18:19
@carlocab carlocab mentioned this pull request Jun 27, 2023
Closed
6 tasks
@carlocab carlocab added the ready to merge PR can be merged once CI is green label Jun 27, 2023
@github-actions github-actions bot removed the CI-long-timeout [DEPRECATED] Use longer GitHub Actions CI timeout. label Jun 27, 2023
@github-actions
Copy link
Contributor

:shipit: @p-linnane has requested bottles to be published to this PR.

@github-actions github-actions bot added the CI-published-bottle-commits The commits for the built bottles have been pushed to the PR branch. label Jun 27, 2023
@BrewTestBot BrewTestBot enabled auto-merge June 27, 2023 21:23
@BrewTestBot BrewTestBot added this pull request to the merge queue Jun 27, 2023
Merged via the queue into Homebrew:master with commit 690db00 Jun 27, 2023
@carlocab carlocab deleted the apache-arrow-protobuf-grpc branch June 27, 2023 23:35
carlocab added a commit to carlocab/netdata that referenced this pull request Jun 27, 2023
@carlocab
Replace info macro with a less generic name
1a45b4d 
The `info` macro is terribly generic, and can cause compilation problems
with other software.

In particular, newer versions of Protobuf (22+) pull in Abseil, which
defines an `info` function[^1] that is broken by the macro defined in
`libnetdata/log/log.h`.

This change will help unblock us at Homebrew, where we are trying to
switch our packages to using a newer version of `protobuf`.[^2]

[^1]: https://github.com/abseil/abseil-cpp/blob/e6c09ae4b2acd421a29706f86e66eaa422262ad0/absl/strings/internal/cordz_update_scope.h#L61
[^2]: Homebrew/homebrew-core#133746
@scpeters
Copy link
Member

thanks for fixing this!

carlocab added a commit to carlocab/netdata that referenced this pull request Jun 29, 2023
@carlocab
Replace info macro with a less generic name
987b7d1 
The `info` macro is terribly generic, and can cause compilation problems
with other software.

In particular, newer versions of Protobuf (22+) pull in Abseil, which
defines an `info` function[^1] that is broken by the macro defined in
`libnetdata/log/log.h`.

This change will help unblock us at Homebrew, where we are trying to
switch our packages to using a newer version of `protobuf`.[^2]

[^1]: https://github.com/abseil/abseil-cpp/blob/e6c09ae4b2acd421a29706f86e66eaa422262ad0/absl/strings/internal/cordz_update_scope.h#L61
[^2]: Homebrew/homebrew-core#133746
@github-actions github-actions bot added the outdated PR was locked due to age label Jul 28, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 28, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@p-linnane p-linnane p-linnane approved these changes

@github-actions github-actions[bot] github-actions[bot] approved these changes

Assignees
No one assigned
Labels
CI-no-fail-fast Continue CI tests despite failing GitHub Actions matrix builds. CI-published-bottle-commits The commits for the built bottles have been pushed to the PR branch. long build Set a long timeout for formula testing no long build conflict Do not allow merging other pull requests when files conflict with this one outdated PR was locked due to age ready to merge PR can be merged once CI is green upstream issue An upstream issue report is needed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@carlocab @ORIOLESFan02 @scpeters @p-linnane @BrewTestBot