gdbm: --enable-libgdbm-compat #56834

rfinnie · 2020-06-24T20:51:21Z

This PR is not blocked or waiting on submitter feedback. Summary of replies and comments made within the last month:

--enable-libgdbm-compat provides gdbm's compatibility ndbm.h library (which is different than POSIX ndbm.h).
This is enabled on every other distribution's gdbm package, except Homebrew. It is of use to developers who need to compile software against gdbm's ndbm variant.
It is renamed to gdbm-ndbm.h to not conflict with macOS's ndbm.h. This is consistent with Debian and Fedora's (and their derivatives') naming, and possibly others.
The build is not failing. The failures are from reverse dependencies, which are failing regardless of this change. The change does not introduce any new failures of reverse dependencies.

--enable-libgdbm-compat for dbm.h / gdbm-ndbm.h compatibility:
https://www.gnu.org.ua/software/gdbm/manual/html_chapter/gdbm_19.html

("compat" in this sense means compatibility with gdbm's older dbm / ndbm implementations, which have differences to POSIX ndbm, if you're wondering "why not just use macOS's ndbm.h?"

Have you followed the guidelines for contributing?
Have you checked that there aren't other open pull requests for the same formula update/change?
Have you built your formula locally with 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 brew install <formula>)?

Two targets are provided, "macos" and "macos-gdbm". "macos" will successfully compile, but it appears macOS's default POSIX ndbm implementation is completely broken (db.dptr void pointer to garbage data). "macos-gdbm" uses gdbm_compat (same as the "linux" target), but that requires a Homebrew patch which is not yet accepted[0]. [0] Homebrew/homebrew-core#56834

rfinnie · 2020-06-25T15:40:20Z

Looks like it installed and tested all children dependencies (which are effectively all formulae). None of the failures were regressions due to this request.

brew test --verbose agda
brew test --verbose alure
brew test --verbose buku
brew install root
brew test --verbose spdylay
brew test --verbose yafc

Two targets are provided, "macos" and "macos-gdbm". "macos" will successfully compile, but it appears macOS's default POSIX ndbm implementation is completely broken (db.dptr void pointer to garbage data). "macos-gdbm" uses gdbm_compat (same as the "linux" target), but that requires a Homebrew patch which is not yet accepted[0]. [0] Homebrew/homebrew-core#56834

rfinnie · 2020-07-01T19:14:45Z

Hi, any update on this? It passes CI (dependent formulae failed for unrelated reasons, see above) and is ready to be reviewed.

SMillerDev · 2020-07-01T19:53:49Z

Does anyone else ship this option enabled by default? And would this be useful to the majority of homebrew users?

rfinnie · 2020-07-01T20:07:36Z

@SMillerDev Yes, it's enabled for every distro I know of (Debian and derivatives, Fedora and derivatives, Arch, etc). And it's a library, so it's of use for people who need to compile software against gdbm's ndbm variant.

rfinnie · 2020-07-09T20:18:21Z

Rebased due to conflicts since this was originally opened. @SMillerDev (or anyone other reviewers), please see reply above, this is still ready to be reviewed.

rfinnie · 2020-07-17T21:23:32Z

Anybody?

rfinnie · 2020-07-25T17:41:50Z

I'm not sure what to do here. @SMillerDev hasn't replied to my response to the questions, this ticket is so far down the list that nobody looks at it (or is hesitant to look at it because if it's so far down the list, it's not low hanging fruit), and per https://docs.brew.sh/How-To-Open-a-Homebrew-Pull-Request the only recourse is:

Post a comment on your pull request if you’ve provided all the requested changes/information and it hasn’t been merged after a week.

SMillerDev · 2020-07-25T19:54:50Z

The build is failing, so that needs to be resolved before we can merge.

rfinnie · 2020-07-25T20:06:53Z

The build is failing, so that needs to be resolved before we can merge.

@SMillerDev Please see above, those are reverse dependencies which are failing unrelated to this change. The build is not failing, nor is the change causing any reverse dependencies to fail.

rfinnie · 2020-07-25T20:30:39Z

I have updated the main description of the summary of the replies I've made in this PR within the last month.

sjackman · 2020-07-28T01:03:16Z

Error: 10 failed steps!
brew test --verbose adios2
brew test --verbose agda
brew test --verbose alure
brew fetch --retry bzt
brew test --verbose dps8m
brew test --verbose ffsend
brew test --verbose howdoi
brew test --verbose ncview
brew test --verbose opentsdb
brew test --verbose yafc
##[error]Process completed with exit code 1.

https://github.com/Homebrew/homebrew-core/pull/56834/checks?check_run_id=855554230#step:6:15319

If you had the interest and inclination, the easiest way to get this PR merged would be to open PRs that fix these ten tests. That's not expected of you at all, but CI passing with a happy ✅ would make this PR way easier to merge. In the mean time, we're discussing this PR and how to deal with these CI failure.

sjackman · 2020-07-28T01:21:14Z

I'm in favour of this PR, CI issues aside.

rfinnie · 2020-07-28T02:09:51Z

Error: 10 failed steps!
brew test --verbose adios2
brew test --verbose agda
brew test --verbose alure
brew fetch --retry bzt
brew test --verbose dps8m
brew test --verbose ffsend
brew test --verbose howdoi
brew test --verbose ncview
brew test --verbose opentsdb
brew test --verbose yafc
##[error]Process completed with exit code 1.
https://github.com/Homebrew/homebrew-core/pull/56834/checks?check_run_id=855554230#step:6:15319

If you had the interest and inclination, the easiest way to get this PR merged would be to open PRs that fix these ten tests. That's not expected of you at all, but CI passing with a happy white_check_mark would make this PR way easier to merge. In the mean time, we're discussing this PR and how to deal with these CI failure.

Some have already been fixed (opentsdb from a quick check, #58076), one had apparently been failing for a long time and was easy to fix (yafc, #58717 filed), but it seems many of them are simply transient failures, usually because of external dependencies. bzt was particularly interesting; looks like in the 20 hours between when these tests started and ended, it was updated and the old bottle disappeared.

As is, it appears any formula with a significant number of reverse dependencies currently need to be manually merged since at least one transient failure is going to happen (and that will start covering up serial problems like yafc -- see e.g. #58302).

Edit: Additionally fixed dps8m (#58720) and alure (#58722).

sjackman · 2020-07-28T03:01:02Z

CI takes 20.5 hours. 😬

sjackman · 2020-07-28T03:02:06Z

@Homebrew/core Any objections to merging this PR, ignoring the CI failures?

MikeMcQuaid · 2020-07-28T07:37:07Z

@Homebrew/core Any objections to merging this PR, ignoring the CI failures?

As-is: yes. The last CI run was 18 days ago. I'd like to see CI rerun and some evidence that at least some of these failures aren't due to this PR. I'd also like to see (as is usual/should be standard practise) the maintainer that merges this create an TODO issue to confirm that these failures are unrelated and work on fixing them.

sjackman · 2020-07-28T16:19:34Z

Changing gdbm.rb is causing 1,738 reverse dependencies to be tested. Ten of these tests (0.6%) fail.

rfinnie · 2020-07-28T16:26:23Z

Changing gdbm.rb is causing 1,738 reverse dependencies to be tested. Ten of these tests (0.6%) fail.

And with extra work, they were all determined to be unrelated to the change.

I've noticed the same thing is going on in #58683 for gettext, with pushback for getting it merged because of unrelated reverse dependency failures. While testing reverse dependencies for regressions would be a good idea, and something like an occasional full rebuild test would be good, this current system is not helping.

sjackman · 2020-07-28T16:27:22Z

The last CI run was 18 days ago. I'd like to see CI rerun and some evidence that at least some of these failures aren't due to this PR.

I agree.
@rfinnie Please rebase this PR onto origin/HEAD. If any of these 10 failures now pass, we know that they were transient failures that we can deal with separately.
Something you can do to help us is brew install and brew test these ten failures on your local machine on HEAD and report the results here.

--enable-libgdbm-compat for dbm.h / gdbm-ndbm.h compatibility: https://www.gnu.org.ua/software/gdbm/manual/html_chapter/gdbm_19.html

rfinnie · 2020-07-28T16:32:48Z

Rebased, see you in 24 hours.

rfinnie · 2020-07-29T15:02:33Z

Error: 6 failed steps!
brew test --verbose agda
brew test --verbose alure
brew fetch --retry bzt
brew test --verbose dps8m
rm -rf /Users/brew/Library/Caches/Homebrew
brew test --verbose yafc

agda: Existing issue, Cabal v1 install deprecation #55253
alure: Fix merged between run starting and ending, alure: Fix test #58722
bzt: Appears to be a problem with bintray not finding the bottle; filed bzt: Bump revision #58814 to rebuild
dps8m: Unreviewed PR, dps8m: Fix test/audit #58720
yafc: Unreviewed PR, yafc: Fix test server #58717

As for the rm -rf, no idea, something to do with go_mod_cache: https://github.com/Homebrew/homebrew-core/pull/56834/checks?check_run_id=919824030#step:6:12728

==> brew test --verbose woboq_codebrowser
==> brew uninstall --force woboq_codebrowser

==> Running Formulae#cleanup_during!
==> rm -rf /Users/brew/Library/Caches/Homebrew
==> FAILED
rm: /Users/brew/Library/Caches/Homebrew/go_mod_cache/pkg/mod/gopkg.in/validator.v2@v2.0.0-20191107172027-c3144fdedc21/LICENSE: Permission denied
rm: /Users/brew/Library/Caches/Homebrew/go_mod_cache/pkg/mod/gopkg.in/validator.v2@v2.0.0-20191107172027-c3144fdedc21/validator.go: Permission denied
rm: /Users/brew/Library/Caches/Homebrew/go_mod_cache/pkg/mod/gopkg.in/validator.v2@v2.0.0-20191107172027-c3144fdedc21/builtins.go: Permission denied
rm: /Users/brew/Library/Caches/Homebrew/go_mod_cache/pkg/mod/gopkg.in/validator.v2@v2.0.0-20191107172027-c3144fdedc21/README.md: Permission denied
rm: /Users/brew/Library/Caches/Homebrew/go_mod_cache/pkg/mod/gopkg.in/validator.v2@v2.0.0-20191107172027-c3144fdedc21/doc.go: Permission denied
rm: /Users/brew/Library/Caches/Homebrew/go_mod_cache/pkg/mod/gopkg.in/validator.v2@v2.0.0-20191107172027-c3144fdedc21/.gitignore: Permission denied
rm: /Users/brew/Library/Caches/Homebrew/go_mod_cache/pkg/mod/gopkg.in/validator.v2@v2.0.0-20191107172027-c3144fdedc21/validator_test.go: Permission denied
rm: /Users/brew/Library/Caches/Homebrew/go_mod_cache/pkg/mod/gopkg.in/validator.v2@v2.0.0-20191107172027-c3144fdedc21/.travis.yml: Permission denied
rm: /Users/brew/Library/Caches/Homebrew/go_mod_cache/pkg/mod/gopkg.in/validator.v2@v2.0.0-20191107172027-c3144fdedc21/examplevalidate_test.go: Permission denied
[...]

As before, none of the failures are related to the change. Please go ahead and merge, thanks.

BrewTestBot · 2020-07-29T15:45:39Z

@MikeMcQuaid has triggered a merge.

MikeMcQuaid · 2020-07-29T15:46:53Z

Thanks so much for your contribution (and patience)! Without people like you submitting PRs we couldn't run this project. You rock, @rfinnie!

rfinnie force-pushed the gdbm-compat branch from 2084762 to ccaec90 Compare July 9, 2020 20:14

gdbm: --enable-libgdbm-compat

bb35156

--enable-libgdbm-compat for dbm.h / gdbm-ndbm.h compatibility: https://www.gnu.org.ua/software/gdbm/manual/html_chapter/gdbm_19.html

rfinnie force-pushed the gdbm-compat branch from ccaec90 to bb35156 Compare July 28, 2020 16:32

BrewTestBot closed this in 923da7c Jul 29, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

gdbm: --enable-libgdbm-compat #56834

gdbm: --enable-libgdbm-compat #56834

rfinnie commented Jun 24, 2020 •

edited

Loading

rfinnie commented Jun 25, 2020 •

edited

Loading

rfinnie commented Jul 1, 2020

SMillerDev commented Jul 1, 2020

rfinnie commented Jul 1, 2020 •

edited

Loading

rfinnie commented Jul 9, 2020

rfinnie commented Jul 17, 2020

rfinnie commented Jul 25, 2020

SMillerDev commented Jul 25, 2020

rfinnie commented Jul 25, 2020

rfinnie commented Jul 25, 2020

sjackman commented Jul 28, 2020

sjackman commented Jul 28, 2020

rfinnie commented Jul 28, 2020 •

edited

Loading

sjackman commented Jul 28, 2020

sjackman commented Jul 28, 2020

MikeMcQuaid commented Jul 28, 2020

sjackman commented Jul 28, 2020

rfinnie commented Jul 28, 2020

sjackman commented Jul 28, 2020

rfinnie commented Jul 28, 2020

rfinnie commented Jul 29, 2020 •

edited

Loading

BrewTestBot commented Jul 29, 2020

MikeMcQuaid commented Jul 29, 2020

gdbm: --enable-libgdbm-compat #56834

gdbm: --enable-libgdbm-compat #56834

Conversation

rfinnie commented Jun 24, 2020 • edited Loading

rfinnie commented Jun 25, 2020 • edited Loading

rfinnie commented Jul 1, 2020

SMillerDev commented Jul 1, 2020

rfinnie commented Jul 1, 2020 • edited Loading

rfinnie commented Jul 9, 2020

rfinnie commented Jul 17, 2020

rfinnie commented Jul 25, 2020

SMillerDev commented Jul 25, 2020

rfinnie commented Jul 25, 2020

rfinnie commented Jul 25, 2020

sjackman commented Jul 28, 2020

sjackman commented Jul 28, 2020

rfinnie commented Jul 28, 2020 • edited Loading

sjackman commented Jul 28, 2020

sjackman commented Jul 28, 2020

MikeMcQuaid commented Jul 28, 2020

sjackman commented Jul 28, 2020

rfinnie commented Jul 28, 2020

sjackman commented Jul 28, 2020

rfinnie commented Jul 28, 2020

rfinnie commented Jul 29, 2020 • edited Loading

BrewTestBot commented Jul 29, 2020

MikeMcQuaid commented Jul 29, 2020

rfinnie commented Jun 24, 2020 •

edited

Loading

rfinnie commented Jun 25, 2020 •

edited

Loading

rfinnie commented Jul 1, 2020 •

edited

Loading

rfinnie commented Jul 28, 2020 •

edited

Loading

rfinnie commented Jul 29, 2020 •

edited

Loading