homebrew-core
homebrew-core copied to clipboard
newnode-helper 2.1.4 (new formula)
- [x] Have you followed the guidelines for contributing?
- [x] Have you ensured that your commits follow the commit style guide?
- [x] Have you checked that there aren't other open pull requests for the same formula update/change?
- [x] 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? - [x] Is your test running fine
brew test <formula>, where<formula>is the name of the formula you're submitting? - [x] Does your build pass
brew audit --strict <formula>(after doingHOMEBREW_NO_INSTALL_FROM_API=1 brew install --build-from-source <formula>)? If this is a new formula, does it passbrew audit --new <formula>?
Newnode is an open-source decentralized Content Distribution Network (https://github.com/clostra/newnode).
This is a rewrite of #153960 with brew services in mind. Tested on intel OSx 14 Sonoma.
Thanks for contributing to Homebrew! :tada: It looks like you're having trouble with a CI failure. See our contribution guide for help. You may be most interested in the section on dealing with CI failures. You can find the CI logs in the Checks tab of your pull request.
This seems to be vendoring a bunch of homebrew formula, which violates: https://docs.brew.sh/Acceptable-Formulae#stuff-that-requires-vendored-versions-of-homebrew-formulae
Entering 'libevent' Entering 'libsodium'
The codebase of newnode uses internal headers of libevent (e.g. in https://github.com/clostra/newnode/blob/master/bufferevent_utp.h), which are not provided in homebrew version. Additionally, our fork of libevent includes important fixes that would lead to errors in the code.
libsodium, however, I think I can try and make it depend on externally.
Would that be a reasonable compromise to resolve this issue?
@SMillerDev any update on this? I can try and look into fixing https://github.com/Homebrew/homebrew-core/pull/162684 (though, I can't promise it), and we're very interested in making the project (d2d vpn with dht) accessible for mac users.
Additionally, our fork of libevent includes important fixes that would lead to errors in the code.
Have they been submitted upstream? Then we can just patch the libevent formula
The fork of libevent we're using has in part been submitted in a series of pull requests (see https://github.com/libevent/libevent/pulls/ghazel), and they might be resubmitted down the line. But this would take a considerable amount of time.
In the meantime, we use features like allow external access to evdns cache, as well as internal headers and ones resulting from compiling libevent (e.g. evconfig.h).
@SMillerDev what do you mean by patching libevent formula, and does it allow for this flexibility?
We've released the new version which should be more compatible with homebrew's policy:
- Upon closer inspection, mbedtls isn't needed, so it is no longer listed as a dependency (neither, the new nor the old version).
- We've updated libevent, which now have some of the functionality similar to that submitted upstream.
- Updated libsodium (we still need it as a controlled dependency for mobile apps).
Does this provide a way forward with this PR?
@SMillerDev are we any closer to completion? It's been over a month since this PR started, and several key concerns have been addressed.
Any update on this? What's missing?
Any update on this? @SMillerDev I believe most comments have been addressed. We do not use liblzma bug in our codebase.
Anything else that need be addressed? @SMillerDev
Any further updates on this? @SMillerDev
Please don't open PR's from organization forks. It prevents maintainers from pushing fixes, and also breaks our merge process.
Please don't open PR's from organization forks. It prevents maintainers from pushing fixes, and also breaks our merge process.
@p-linnane sure, we can either allow access to avoid re-creating this PR or open it from a personal account. Sorry, I didn't realize there was an issue.
Thanks for the PR, but I don't think we can look past the vendor-ing of multiple pieces of software that are provided by Homebrew.
There are already plenty of existing formulae that do this. We're trying to fix them so they stop doing that, but adding another formula that does it just makes that job harder.
Apologies for the extended review process here. We still appreciate the work you put in. I've left suggestions that will help you improve the formula for when it's ready for inclusion into Homebrew/core.
If you wish, you can host this formula in your own tap until then. Here are some docs to help you get started:
https://docs.brew.sh/Taps https://docs.brew.sh/Interesting-Taps-and-Forks https://docs.brew.sh/How-to-Create-and-Maintain-a-Tap https://brew.sh/2020/11/18/homebrew-tap-with-bottles-uploaded-to-github-releases/
@carlocab if we excluded libsodium upstream, in a fork or in a branch, would that be more acceptable? I believe prior communication on that matter left an impression that two vendored formulae was a permissible compromise, and other issues had to be ironed out first. If that's the case, let's reopen it and we will find a way to reduce the load on homebrew maintainers.
@carlocab @SMillerDev should I move these questions somewhere else, e.g. discussions/issues? I understand the concern, but I don't know how to resolve it without some back-and-forth given that the same newnode repo is used for building ios and android packages too. We really want the package to be included but we don't want to spend your time and mental energy figuring out too many nuances.
Essentially we don't want to vendor anything. If the software was super popular (think Chrome or Nginx) we might lift that restriction, but since this tool is not I'd suggest upstreaming your fixes and then you can try again.
Or, as other maintainers pointed out, make a tap.