-
-
Notifications
You must be signed in to change notification settings - Fork 12.4k
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
newnode 2.1.4 (new formula) #153960
newnode 2.1.4 (new formula) #153960
Conversation
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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
adds dependency on macos high sierra so it won't try to build on linux
Why shouldn't it? |
plist_path = prefix/"newnode-helper.plist" | ||
plist_content = <<~EOS | ||
<?xml version="1.0" encoding="UTF-8"?> | ||
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd"> | ||
<plist version="1.0"> | ||
<dict> | ||
<key>Label</key> | ||
<string>com.clostra.newnode-helper</string> | ||
<key>ProgramArguments</key> | ||
<array> | ||
<string>#{opt_bin}/newnode-helper</string> | ||
<string>-p</string> | ||
<string>8006</string> | ||
<string>-v</string> | ||
</array> | ||
<key>RunAtLoad</key> | ||
<true/> | ||
<key>KeepAlive</key> | ||
<true/> | ||
<key>UserName</key> | ||
<string>newnode</string> | ||
<key>WorkingDirectory</key> | ||
<string>/var/newnode-helper</string> | ||
<key>StandardOutPath</key> | ||
<string>/var/log/newnode-helper.log</string> | ||
<key>StandardErrorPath</key> | ||
<string>/var/log/newnode-helper-error.log</string> | ||
</dict> | ||
</plist> | ||
EOS | ||
plist_path.write(plist_content) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this isn't provided by upstream we shouldn't ship it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Way too much magic going on in this install
On 11/12/23 06:17, Sean Molenaar wrote:
If this isn't provided by upstream we shouldn't ship it
Since the upstream git repo is ours, we can fix it to provide this plist.
Keith
|
Separately from the effort to make newnode-helper easily installable via homebrew, we've set up a snap to facilitate easy installation on linux platforms. We judged (perhaps incorrectly) snap to be more likely to facilitate installs on a wide variety of Linux platforms, and homebrew to be more likely to facilitate installs for MacOS because homebrew is very widely used on MacOS. We could adapt this homebrew formula to facilitate homebrew installs on Linux also, since the C source code for newnode builds fine on Linux. But at least in my experience things can get confusing for users when the same software package can be installed via multiple mechanisms. For example, when version X of the software is installed via one mechanism and upgraded to version Y via a different mechanism. So if we wanted to facilitate installs of newnode-helper via both homebrew and snap, we'd want to make sure that all future upgrades (by whatever mechanism) left the resulting installation in a consistent state no matter which mechanism was used to do the upgrade. And that seems tricky, and at least, not what we wanted to try on first attempt. |
Just to let people know, the authors' intention for the newnode-helper formula is to update the underlying repo to put more of the "magic" in the newnode repository code, and less of the "magic" in the homebrew formula, and submit another PR once the new combination is tested. In some ways this is more complicated because it requires coordination among more parties and makes testing more difficult. But we understand that homebrew maintainers want to keep formulae as simple and straightforward as possible. |
That seems like the best solution since the current split seems very arbitrary. I see the risk of https://xkcd.com/1987/, but that's up to users to fix. Not up to us to restrict. |
The split isn't arbitrary, since MacOS and Linux are very different platforms, and most MacOS users are accustomed to getting their software from different sources than most Linux users. Also, what we need for newnode-helper is for the app to run as a daemon full time, and as its own user (least privilege). There's no portable way to do that on Linux. Snap provides a way to do that across a wide variety of Linux platforms. If homebrew on Linux supports the ability to do that on a large number of Linux platforms, please let me know what it is. But I'm under the impression that homebrew really isn't designed for that use case, and it takes "magic" to get it to work even on a Mac. |
It's arbitrary because the tool works for both, but as a package manager we're disabling that choice. It's like buying a car in two countries and in one of them the car will refuse to drive on highways.
|
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. |
Yes, that's definitely the way to go forward. I am closing this inactive PR, but we'll be happy to consider a new one once the install is streamlined. Thanks @flyinhome ! |
HOMEBREW_NO_INSTALL_FROM_API=1 brew install --build-from-source <formula>
, where<formula>
is the name of the formula you're submitting?brew test <formula>
, where<formula>
is the name of the formula you're submitting?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>
?