newnode 2.1.5 (new formula) #162683

theoden8 · 2024-02-14T13:43:28Z

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

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.

Formula/n/newnode-helper.rb

github-actions · 2024-02-14T13:59:41Z

Thanks for contributing to Homebrew! 🎉 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.

Formula/n/newnode-helper.rb

SMillerDev

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'

theoden8 · 2024-02-14T15:20:50Z

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?

theoden8 · 2024-02-21T14:40:25Z

@SMillerDev any update on this? I can try and look into fixing #162684 (though, I can't promise it), and we're very interested in making the project (d2d vpn with dht) accessible for mac users.

SMillerDev · 2024-02-21T14:42:05Z

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

theoden8 · 2024-02-21T16:58:37Z

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?

p-linnane

Thanks for the PR. I have some concerns I need addressed before we attempt to move forward with this.

Formula/n/newnode-helper.rb

further inspection of https://github.com/clostra/newnode/blob/master/build.sh and subsequent build tests confirm that wget isnt needed on mac os

instead of assuming that the proxy is already run as a service, run it in a fork and kill at completion

linux builds currently rely on a hard-coded wget binary, which needs to change upstream to non-hardcoded binary

fix brew audit fail due to non-alphabetic ordering of dependencies

most of the dependencies specified so far are actually build-time only, and pkg-config was not specified at all

updated newnode-helper to 2.1.5, with updated dependencies and no dependency on mbedtls

theoden8 · 2024-02-29T12:56:30Z

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?

Formula/n/newnode-helper.rb

Co-authored-by: Sean Molenaar <SMillerDev@users.noreply.github.com>

theoden8 · 2024-03-12T15:18:02Z

@SMillerDev are we any closer to completion? It's been over a month since this PR started, and several key concerns have been addressed.

theoden8 · 2024-03-19T11:23:55Z

Any update on this? What's missing?

Formula/n/newnode.rb

Co-authored-by: Sean Molenaar <SMillerDev@users.noreply.github.com>

theoden8 · 2024-04-09T09:03:32Z

Any update on this? @SMillerDev I believe most comments have been addressed.
We do not use liblzma bug in our codebase.

SMillerDev · 2024-04-11T05:57:02Z

Formula/n/newnode.rb

+      begin
+        Timeout.timeout(5) do
+          Process.wait(pid)
+        end
+      rescue Timeout::Error
+        Process.kill("KILL", pid) # Forcefully kill if not terminated after timeout
+        Process.wait(pid)
+      end


Why would we expect it not to be killed by the kill above?

Removed the second kill

SMillerDev · 2024-04-11T05:58:32Z

Formula/n/newnode.rb

+    path = (var/"cache/newnode")
+    path.mkpath
+    path.chmod 0775


This means that every install/update will make a new cache. Is that desirable?

can you point us the installation doc reference on this?

Doesn't it reuse the old cache?
https://github.com/clostra/newnode/blob/master/client.c#L3590 this is the closest to an installation doc I can find.

No, currently it will make a new cache folder as part of the install and overwrite the old one.

I believe that's even better! :)

And if people are using the tool when running brew upgrade it'll keep working?

It should, if peers file is invalid it just won't be loaded. There's no parsing, just raw ip addresses. The file is updated when the peer list is updated. if it fails to open file for writing, it will skip saving.

Either way works, saving or not saving. We will handle the update smoothly if that interface ever changes. The code is intentionally simple and shouldn't fail.

Formula/n/newnode.rb

theoden8 · 2024-05-01T14:03:10Z

Anything else that need be addressed? @SMillerDev

newnode-helper 2.1.4 (new formula)

cc01a26

github-actions bot added the new formula PR adds a new formula to Homebrew/homebrew-core label Feb 14, 2024

chenrui333 reviewed Feb 14, 2024

View reviewed changes

Formula/n/newnode-helper.rb Outdated Show resolved Hide resolved

chenrui333 reviewed Feb 14, 2024

View reviewed changes

Formula/n/newnode-helper.rb Outdated Show resolved Hide resolved

chenrui333 reviewed Feb 14, 2024

View reviewed changes

Formula/n/newnode-helper.rb Outdated Show resolved Hide resolved

filled in the license

9d53b49

github-actions bot added the autosquash Automatically squash pull request commits according to Homebrew style. label Feb 14, 2024

removed extra cloning

1003532

SMillerDev reviewed Feb 14, 2024

View reviewed changes

Formula/n/newnode-helper.rb Outdated Show resolved Hide resolved

Formula/n/newnode-helper.rb Outdated Show resolved Hide resolved

SMillerDev requested changes Feb 14, 2024

View reviewed changes

p-linnane requested changes Feb 21, 2024

View reviewed changes

Formula/n/newnode-helper.rb Outdated Show resolved Hide resolved

Formula/n/newnode-helper.rb Outdated Show resolved Hide resolved

Formula/n/newnode-helper.rb Outdated Show resolved Hide resolved

theoden8 added 3 commits February 21, 2024 22:58

removed wget dependency

4984c90

further inspection of https://github.com/clostra/newnode/blob/master/build.sh and subsequent build tests confirm that wget isnt needed on mac os

fork proxy in test

669f2db

instead of assuming that the proxy is already run as a service, run it in a fork and kill at completion

disable linux builds for now

3d132b8

linux builds currently rely on a hard-coded wget binary, which needs to change upstream to non-hardcoded binary

github-actions bot added the macos-only Formula depends on macOS label Feb 22, 2024

theoden8 added 3 commits February 22, 2024 00:51

fix ordering

3fdbb12

fix brew audit fail due to non-alphabetic ordering of dependencies

added build dependencies

5777bd3

most of the dependencies specified so far are actually build-time only, and pkg-config was not specified at all

update to 2.1.5, no need for mbedtls

1ece97d

updated newnode-helper to 2.1.5, with updated dependencies and no dependency on mbedtls

theoden8 changed the title ~~newnode-helper 2.1.4 (new formula)~~ newnode-helper 2.1.5 (new formula) Feb 29, 2024

theoden8 requested review from p-linnane and SMillerDev March 1, 2024 09:48

SMillerDev reviewed Mar 4, 2024

View reviewed changes

Formula/n/newnode-helper.rb Outdated Show resolved Hide resolved

Formula/n/newnode-helper.rb Outdated Show resolved Hide resolved

Formula/n/newnode-helper.rb Outdated Show resolved Hide resolved

Formula/n/newnode-helper.rb Outdated Show resolved Hide resolved

theoden8 and others added 2 commits March 4, 2024 14:25

Update Formula/n/newnode-helper.rb

a726984

Co-authored-by: Sean Molenaar <SMillerDev@users.noreply.github.com>

renamed newnode-helper.rb to newnode.rb

032c4d8

theoden8 changed the title ~~newnode-helper 2.1.5 (new formula)~~ newnode 2.1.5 (new formula) Mar 4, 2024

p-linnane removed their request for review March 4, 2024 18:42

forgot to rename class

47a4614

theoden8 requested review from SMillerDev and p-linnane March 18, 2024 09:40

theoden8 requested a review from chenrui333 March 21, 2024 15:11

SMillerDev removed their request for review March 26, 2024 09:03

theoden8 added 3 commits March 26, 2024 13:32

change working directory to cache/newnode

92b2426

fix cache mkpath

6fd2073

enable linux and replace wget with homebrews wget

bef4833

github-actions bot removed the macos-only Formula depends on macOS label Mar 26, 2024

theoden8 added 3 commits March 26, 2024 14:04

fix audit

480e96e

fix linux builds

83028ed

fix audit

69533d0

p-linnane removed their request for review March 26, 2024 16:12

SMillerDev reviewed Mar 27, 2024

View reviewed changes

Formula/n/newnode.rb Outdated Show resolved Hide resolved

Formula/n/newnode.rb Outdated Show resolved Hide resolved

Update Formula/n/newnode.rb

73f531a

Co-authored-by: Sean Molenaar <SMillerDev@users.noreply.github.com>

SMillerDev reviewed Apr 11, 2024

View reviewed changes

chenrui333 reviewed Apr 11, 2024

View reviewed changes

Formula/n/newnode.rb Outdated Show resolved Hide resolved

theoden8 added 4 commits April 11, 2024 13:31

formatted revision

f7160d8

no need to double kill

b57c191

fix audit

d282a5e

fails_with :gcc

42a477f

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

newnode 2.1.5 (new formula) #162683

newnode 2.1.5 (new formula) #162683

theoden8 commented Feb 14, 2024 •

edited

github-actions bot commented Feb 14, 2024

SMillerDev left a comment

theoden8 commented Feb 14, 2024 •

edited

theoden8 commented Feb 21, 2024 •

edited

SMillerDev commented Feb 21, 2024

theoden8 commented Feb 21, 2024 •

edited

p-linnane left a comment

theoden8 commented Feb 29, 2024 •

edited

theoden8 commented Mar 12, 2024 •

edited

theoden8 commented Mar 19, 2024

theoden8 commented Apr 9, 2024

SMillerDev Apr 11, 2024

theoden8 Apr 11, 2024

SMillerDev Apr 11, 2024

chenrui333 Apr 11, 2024

theoden8 Apr 11, 2024

SMillerDev Apr 11, 2024

theoden8 Apr 11, 2024 •

edited

SMillerDev Apr 11, 2024

theoden8 Apr 11, 2024 •

edited

theoden8 Apr 29, 2024 •

edited

theoden8 commented May 1, 2024 •

edited

newnode 2.1.5 (new formula) #162683

Are you sure you want to change the base?

newnode 2.1.5 (new formula) #162683

Conversation

theoden8 commented Feb 14, 2024 • edited

github-actions bot commented Feb 14, 2024

SMillerDev left a comment

Choose a reason for hiding this comment

theoden8 commented Feb 14, 2024 • edited

theoden8 commented Feb 21, 2024 • edited

SMillerDev commented Feb 21, 2024

theoden8 commented Feb 21, 2024 • edited

p-linnane left a comment

Choose a reason for hiding this comment

theoden8 commented Feb 29, 2024 • edited

theoden8 commented Mar 12, 2024 • edited

theoden8 commented Mar 19, 2024

theoden8 commented Apr 9, 2024

SMillerDev Apr 11, 2024

Choose a reason for hiding this comment

theoden8 Apr 11, 2024

Choose a reason for hiding this comment

SMillerDev Apr 11, 2024

Choose a reason for hiding this comment

chenrui333 Apr 11, 2024

Choose a reason for hiding this comment

theoden8 Apr 11, 2024

Choose a reason for hiding this comment

SMillerDev Apr 11, 2024

Choose a reason for hiding this comment

theoden8 Apr 11, 2024 • edited

Choose a reason for hiding this comment

SMillerDev Apr 11, 2024

Choose a reason for hiding this comment

theoden8 Apr 11, 2024 • edited

Choose a reason for hiding this comment

theoden8 Apr 29, 2024 • edited

Choose a reason for hiding this comment

theoden8 commented May 1, 2024 • edited

theoden8 commented Feb 14, 2024 •

edited

theoden8 commented Feb 14, 2024 •

edited

theoden8 commented Feb 21, 2024 •

edited

theoden8 commented Feb 21, 2024 •

edited

theoden8 commented Feb 29, 2024 •

edited

theoden8 commented Mar 12, 2024 •

edited

theoden8 Apr 11, 2024 •

edited

theoden8 Apr 11, 2024 •

edited

theoden8 Apr 29, 2024 •

edited

theoden8 commented May 1, 2024 •

edited