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

gensio 2.8.5 (new formula) ser2net: update to use gensio formula #169138

Merged
merged 4 commits into from
Jun 11, 2024

Conversation

cminyard
Copy link
Contributor

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

I submitted this earlier, before gensio had enough likes and forks. It now does. All audits pass.

The ser2net formula builds gensio as part of its build, but gensio is a separate stand-alone library and set of tools. This commit splits gensio into its own separate formula so it can be properly maintained, and makes ser2net depend on that.

As well, the ser2net formula was non-functional. It had the following issues:

  • The configuration files were set to inside the /opt/homebrew/etc/ser2net, so every time you upgraded you would lose your configuration. This change moved it to /opt/homebrew/etc/ser2net.

  • Same problem with the authentication token location, that is moved to /opt/homebrew/share.

  • The service was run in the background, so that caused all kinds of confusion. This switches it to run in the foreground so brew can manage it.

  • The administration interface was enabled on the command line. This has security implications and is generally a bad idea outside of testing. The admin interface can be enabled in the configuration files with the proper authentication.

@github-actions github-actions bot added the new formula PR adds a new formula to Homebrew/homebrew-core label Apr 16, 2024
Formula/s/ser2net.rb Outdated Show resolved Hide resolved
Formula/g/gensio.rb Outdated Show resolved Hide resolved
Formula/g/gensio.rb Outdated Show resolved Hide resolved
Formula/g/gensio.rb Outdated Show resolved Hide resolved
Formula/g/gensio.rb Outdated Show resolved Hide resolved
@cminyard
Copy link
Contributor Author

cminyard commented Apr 16, 2024 via email

@SMillerDev
Copy link
Member

There is a tag on GitHub, but traditionally I release the tarballs on sourceforge because it's been there a long time and that's where everyone expects it. I can add it to GitHub, if you like.

If the homepage is GitHub I would expect the code to also come from there. If there is a tag we can just download the tarball from there.

@cminyard
Copy link
Contributor Author

cminyard commented Apr 16, 2024 via email

@cminyard
Copy link
Contributor Author

I have made all the requested modifications, I think it's ready for another review.

Formula/g/gensio.rb Outdated Show resolved Hide resolved
Formula/g/gensio.rb Show resolved Hide resolved
@github-actions github-actions bot added the automerge-skip `brew pr-automerge` will skip this pull request label Apr 21, 2024
Copy link
Contributor

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.

@cminyard cminyard force-pushed the fix-ser2net2 branch 2 times, most recently from a29e252 to 0aeb1c1 Compare April 21, 2024 12:35
@github-actions github-actions bot removed the automerge-skip `brew pr-automerge` will skip this pull request label Apr 21, 2024
@cminyard
Copy link
Contributor Author

Ok, one more time. Hopefully I have everything, I don't see anything else.

@chenrui333 chenrui333 changed the title Fix ser2net by splitting out gensio and fixing issues gensio 2.8.4 (new formula) ser2net: update to use gensio formula Apr 29, 2024
@cminyard
Copy link
Contributor Author

Anything else I need to do here? It's been three weeks without any real comments.

The gensio library is a library to make stream I/O easy.  It has all kinds
of tools, including encryption, authentication.  ser2net, a program for
providing network access to serial ports, requires it.  This adds the
library as a new Forumla.

Signed-off-by: Corey Minyard <[email protected]>
Now that gensio is a separate formula, remove all the build of gensio
in ser2net and make it depend on gensio.

Put the ser2net control directory in /opt/homebrew/etc/ser2net instead of
being in the ser2net directory.

Set the datarootdir, where ser2net stores it authentication information,
to /opt/homebrew/share/ser2net.

Run the service in forground as it should be.

Don't enable the admin interface through the command line, the user
can use ser2net.yaml to enable it if they want.

Signed-off-by: Corey Minyard <[email protected]>
@chenrui333 chenrui333 changed the title gensio 2.8.4 (new formula) ser2net: update to use gensio formula gensio 2.8.5 (new formula) ser2net: update to use gensio formula Jun 11, 2024
Copy link
Member

@chenrui333 chenrui333 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can followup on the audit failures

@chenrui333 chenrui333 added the ready to merge PR can be merged once CI is green label Jun 11, 2024
@carlocab carlocab added this pull request to the merge queue Jun 11, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jun 11, 2024
Copy link
Contributor

🤖 An automated task 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 11, 2024
@BrewTestBot BrewTestBot added this pull request to the merge queue Jun 11, 2024
Merged via the queue into Homebrew:master with commit 7589ce2 Jun 11, 2024
14 checks passed
@timsutton timsutton mentioned this pull request Jun 12, 2024
1 task
@github-actions github-actions bot added the outdated PR was locked due to age label Jul 12, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 12, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CI-published-bottle-commits The commits for the built bottles have been pushed to the PR branch. new formula PR adds a new formula to Homebrew/homebrew-core outdated PR was locked due to age ready to merge PR can be merged once CI is green
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants