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

alpine: add option for compiling with Maildir mailbox support #128

Closed
wants to merge 1 commit into from

Conversation

mbarnett
Copy link

@mbarnett mbarnett commented Apr 7, 2016

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same formula update/change?
  • Does your submission pass brew audit --strict --online <formula> (where <formula> is the name of the formula you're submitting)?
  • Have you built your formula locally prior to submission with brew install <formula>?

Adds an option to compile alpine with the official Maildir patch included.

@apjanke
Copy link
Contributor

apjanke commented Apr 7, 2016

Hmmm. We tend to avoid patches in Homebrew core formula unless needed to fix a build problem or OS X compatibility issue. This one just enables new functionality. The http://patches.freeiz.com/alpine/ place contains a bunch of patches to Alpine, and isn't the original distributor of Alpine, which is http://www.washington.edu/alpine/acquire/. They stopped at version 2.00; this freeiz.com looks like a fork.

There's a bunch of other patches there that freeiz Alpine users might want to make use of. Maybe this would make more sense in a dedicated freeiz-alpine tap somewhere maintained by some freeiz Alpine aficionados, instead of in core Homebrew? Then you would have free reign to throw in all the options and patches people are interested in.

Thoughts from other maintainers?

@apjanke apjanke added the marked for removal/rejection PR is probably going to be closed or formula deleted label Apr 7, 2016
@mbarnett
Copy link
Author

mbarnett commented Apr 7, 2016

If I'm understanding you correctly, that would involve moving alpine out of homebrew-core entirely, since the current version in -core is the 2.20 fork coming from http://patches.freeiz.com/alpine/?

Personally I have zero interest in maintaining a tap, so if any interested parties out there would find this useful, feel free to run with it.

Edit: Just to summarize my understanding of the situation: upstream is http://patches.freeiz.com/alpine/ for both this package AND the patch. This option would be acceptable if upstream made it a build option on http://patches.freeiz.com/alpine/release/src/alpine-2.20.tar.xz rather than distributing it as a separate patch. Because upstream chooses not to do that, for whatever reason, it is suggested that a separate tap should be created for all separate-patch options provided by upstream, (and that -core should cease providing alpine entirely?).

@apjanke
Copy link
Contributor

apjanke commented Apr 7, 2016

I think this Alpine could stay in core, and just not take any patches or options. Forks are fine; it's opening up all the patches and variants that's not well suited for core. (Since it is a fork, IMHO it would be better named freeiz-alpine or alpine-freeiz, with alpine being the 2.00 original one. But it's already using this name.) If someone wanted to make all those variants available, they could make a tap that had a freeiz-alpine with a bunch of options and variants. We would either keep the basic version in core as a convenience for users, or remove it in favor of the new tap's formula, probably at their discretion.

Do you know how active the Freeiz Alpine community is and how stable these "patches" are? If it's actively maintained, we might consider this more of an "optional component" than a "patch" and pull it in regardless, like we do with "resources" in other formulae.

Maybe Freeiz themselves would want to run a tap. If they use OS X, it can be convenient for managing installations for testing. And if you maintain your own tap, you can include different versions of the formula in it, too.

@mbarnett
Copy link
Author

mbarnett commented Apr 7, 2016

The extent of my understanding is that UW/alpine is essentially dead as of 2.00-ish. The realpine project picked it up through version 2.11, and then died off itself. Eduado Chappa ("freeiz") then picked up maintainer-ship from there, and released 2.20. The "freeiz-alpine" community is indistinguishable from the "alpine" community in general at this point (judging mostly by comp.mail.pine, where Edduardo is treated as the defacto maintainer of the project), since that is the only place bugs in the old UW/realpine releases are being fixed.

Stability probably varies by patch. The maildir patchset runs back a decade or more. From what I understand it was originally distributed as a patch because of licensing terms UW imposed on the pine source. Since the license changed with alpine I don't have any real insight as to why it continues to be distributed separately rather than folded into the source.

@apjanke apjanke self-assigned this Apr 7, 2016
@apjanke
Copy link
Contributor

apjanke commented Apr 7, 2016

Gotcha. In that case, this makes sense to have the alpine name.

Having some of the patches included in the core repo's version of Alpine may or may not make sense then. Since they are "patches", I'd like to hold off on accepting this until I have a chance to do some reading and get more familiar with the Freeiz approach, or some other maintainer or contributor who knows Alpine better can chime in.

I should have some time to do so soon, so let's just leave this PR open as a placeholder for now. I've self-assigned.

@apjanke apjanke added in progress Stale bot should stay away and removed marked for removal/rejection PR is probably going to be closed or formula deleted labels Apr 7, 2016
patch do
url "http://patches.freeiz.com/alpine/patches/alpine-2.20/maildir.patch.gz"
sha256 "1ef0932b80d7f790ce6577a521a7b613b5ce277bb13cbaf0116bb5de1499caaa"
end if build.with? "maildir"
Copy link
Contributor

Choose a reason for hiding this comment

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

Though it is more lines, current style prefers

if build.with? "blah"
  patch do
    url ""
    sha256 ""
  end
end

Trailing conditionals on multi-line statements are easily overlooked.

@MikeMcQuaid
Copy link
Member

Passing on this patch; these should be included in the upstream software or, if it's unmaintained, a new maintained version of the software created which can include such patches. Sorry!

@MikeMcQuaid MikeMcQuaid closed this Apr 8, 2016
@mbarnett
Copy link
Author

mbarnett commented Apr 8, 2016

This patch is distributed by upstream, so I can't pass it on to someone who already has it. Upstream, for whatever reason, chooses to use patches to provide options in builds.

But it's clear that upstream doesn't behave the way homebrew wants the world to work, so ok.

dunn pushed a commit to dunn/homebrew-core that referenced this pull request Aug 29, 2016
postgres-xc was trying to build with Bonjour. Also, it was missing a few
dependencies.

Closes Linuxbrew/homebrew-core#127.
@Homebrew Homebrew locked and limited conversation to collaborators May 4, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
in progress Stale bot should stay away
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants