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

Brew formula #1275

Merged
merged 20 commits into from
Dec 28, 2024
Merged

Brew formula #1275

merged 20 commits into from
Dec 28, 2024

Conversation

dgreatwood
Copy link
Collaborator

This PR adds the Homebrew formula for Pistache to the Pistache source tree. This is the formula used by the popular brew package manager to install Pistache on macOS.

Updated files:

  • README.md: Updated to reflect the option of a brew install in macOS
  • Building on macOS.txt: Updated to reflect the option of a brew install
  • meson.build: Deal with differing howard-hinnant-date names (required by brew)
  • tests/meson.build: Allow use of subproject httplib.h with brew
  • rest_server_test.cc, rest_swagger_server_test.cc, router_test.cc: Suppress httplib.h GCC warning (causing build issue on Ubuntu). Also, removed some whitespace.

New files:

  • pistache.rb: This is the brew formula
  • runformula.sh: A script for developers, to install or test the Pistache brew formula
  • brew.yaml: GitHub workflow to test the brew formula

dgreatwood and others added 19 commits December 23, 2024 16:03
Also:

Allow "howard-hinnant-date" for dependency name to address
brew-install issue when creating a homebrew formula

Make homebrew formula pass brew audit
 - Sorted the dependencies into the order required by brew audit
 - Switched to using tarball (not zip) for release

runformula.sh: (script to install pistache brew formula)
 - Allow "--HEAD" as a parameter
 - Add "skipaudit" option to runformula.sh
 - Provide/update usage/help message for runformula.sh
 - Provide --force Option on runformula.sh
 - Provide specific output regarding which formula file is being
   copied where.

Created brew.yaml (github runner for the brew formula)
bldscripts/gccmacsetup.sh
  Added this script, which configures gcc on macOS

pist_syslog.cc
  Uses syslog for gcc on macOS. The macOS logging doesn't compile
  under gcc.
  Removed some trailing whitespace

listener.cc
  Suppress a warning in openssl/ssl.h that shows up with gcc-on-macOS

router.cc
  Avoid allocating a zero-sized array (not allowed by gcc)

http_server_test.cc
  Explicitly sign a "0" to avoid integer comparison warning
  Removed some trailing whitespace

mime_test.cc
  Handle a locale exception that can happen with gcc on macOS

macos.yaml
  Fix: Use gccmacsetup.sh in macos.yaml for GCC Builds

Convenience Build Scripts (bldscripts/*)
  - Copied the revised versions from the "windows" branch. This moves
    the helper scripts (helpers to the main scripts) into a
    subdirectory "helpers". Merged in the local changes from this
    branch.

Building on macOS.txt:
  Updated regarding building with GCC

fd_utils.cc:
  Address integer-size warning in fd_utils.cc

version.txt
  Update Version
Newer headers (like pist_strerror_r.h) added to install_headers in
/include/pistache/meson.build.
winornix.h, pist_strerror_r.h, pist_strerror_r.cc: Wrap strerror_r for
Apple GCC case (Apple GCC defaults to XSI strerror_r, which is not the
strerror_r we want). Fixes compile error for GCC on macOS.

Also:
winornix.h: we wrap errno.h with pst_errno.h in all non-Linux
cases. Fixes a compile issue on openBSD.
There were 4 issues (1 error, 3 warnings) that showed up in the
Launchpad Arm-32 build following merge of the Windows-support PR, all
to do with integer sign and width conversions. Addressing those issues
here.
Building on macOS.txt
 - Update to reflect brew install

Also: pistache.rb updated for latest Pistache version
Void static initialization order fiasco in case an application itself
uses statics which depend on the library.
Use strings in include strings definitions to avoid replacements by
macros. This solves the issues with errno.h which could have end up in
strange include errors, before.
Make sure, that every compilation unit has an include to pist_quote.h
where needed. Additionally removed dupplicate includes.
Removed some comment lines that I presume were added during earlier debugging.
* GitHub fully deprecated support for their macOS 12 image as of Dec
3rd 2024 per: actions/runner-images#10721
Allow fallback for httplib on macos and linux even with nofallback

On macOS, meson doesn't seem to find cpp-httplib even when cpp-httplib
has been installed in the system (i.e. even when cpp-httplib has been
installed with brew); I don't know why. This causes a particular issue
with Pistache's brew formula, since brew's "std_meson_args", used by
the formula, includes "--wrap-mode=nofallback", preventing meson from
using our httplib subproject as a fallback (brew wants us to use brew
to install each dependency, in preference to using a subproject). We
workaround the issue in tests/meson.build by effectively doing the
fallback to the cpp-httplib subproject without using meson
dependency's "fallback:" option.

Plus, follow the above logic on Linux when --wrap-mode=nofallback is
set, to enable linuxbrew.

Also:
 - Enable Homebrew to find libcurl on linux
 - Using release Pistache for the formula

Updates after Homebrew Review:

pistache.rb:
- Correctly identify which dependency are build-only
- Add zlib and libcurl as dependencies for linuxbrew
- Remove redundant --buildtype=release
- Update to Pistache release v0.4.26

runformula.sh:
- Drop the copyright+license SPDX message from pistache.rb when
  copying pistache.rb to homebrew-core folder

brew.yaml
 - Remove macOS-12 from brew.yaml
pist_syslog.cc
 - There was a duplicate basename_r here, probably a result of an
   automated merge issue. Removed.

rest_server_test.cc
rest_swagger_server_test.cc
router_test.cc
 - Suppress gcc "unused-parameter" warning for httplib.h, causing
   Ubuntu build issue.
Also: Add Comment About GitHub Rate Limit in brew.yaml
Copy link

codecov bot commented Dec 26, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 77.37%. Comparing base (eed02c1) to head (a96e084).
Report is 22 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1275      +/-   ##
==========================================
+ Coverage   77.34%   77.37%   +0.02%     
==========================================
  Files          57       57              
  Lines        7659     7664       +5     
==========================================
+ Hits         5924     5930       +6     
+ Misses       1735     1734       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

README.md Outdated
@@ -103,7 +107,7 @@ $ sudo apt update
$ sudo apt install libpistache-dev
```

#### Ubuntu PPA (Stable)
### Ubuntu PPA (Stable)

Currently there are no stable release of Pistache published into the [stable](https://launchpad.net/~pistache+team/+archive/ubuntu/stable) PPA. However, when that time comes, run the following to install a stable package:
Copy link
Member

Choose a reason for hiding this comment

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

@dgreatwood, this isn't really an issue you created, but feel free to fix this paragraph to reflect the fact that we now have stable releases in the stable PPA.

@dgreatwood
Copy link
Collaborator Author

dgreatwood commented Dec 26, 2024 via email

Copy link
Member

@Tachi107 Tachi107 left a comment

Choose a reason for hiding this comment

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

Two small comments :)

@@ -0,0 +1,91 @@
# SPDX-FileCopyrightText: 2022 Andrea Pappacoda
Copy link
Member

Choose a reason for hiding this comment

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

I didn't write this, I do not own copyright on the file :)

meson.build Outdated Show resolved Hide resolved
@dgreatwood
Copy link
Collaborator Author

dgreatwood commented Dec 27, 2024 via email

@dgreatwood dgreatwood force-pushed the brewFormula branch 2 times, most recently from 1cfbb4f to acf2126 Compare December 28, 2024 02:05
brew.yaml was commonly hitting a GitHub rate limit by doing
unauthenticated GitHub access.

We have reduced unauthenticated access in brew.yaml in two ways:
  1/ For brew install and test phases, we are providing an access
  token to brew via the HOMEBREW_GITHUB_API_TOKEN environment
  variable. We use a fine-grained personal access token restricted to
  a single repository and with all permissions left on "No Access".
and
  2/ For brew audit, we cannot use the access token described above -
  since the brew audit objects to it. So we have simply eliminated
  doing a brew audit from the workflow.

Taken together, these measures seem to be enough to avoid the rate
limit issue.

homebrew/runformula.sh: Added the "--auditonly" option (but this is
not used in the workflow).

Also: Update README to Reflect Stable PPA Versions
@kiplingw kiplingw merged commit 11738a5 into pistacheio:master Dec 28, 2024
136 of 137 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants