Opened 6 years ago

Closed 4 years ago

Last modified 4 years ago

#38091 closed update (fixed)

alpine @2.00_4: update to 2.11

Reported by: larryv (Lawrence Velázquez) Owned by: jcvernaleo (John C. Vernaleo)
Priority: Normal Milestone:
Component: ports Version:
Keywords: haspatch Cc: macosforge.org@…, jummo4@…, patrick.sizun@…, SickTeddyBear, ryandesign (Ryan Schmidt)
Port: alpine

Description

Alpine 2.10 has been released. We should probably update the alpine port.

Attachments (8)

alpine-2.10_patch.diff (1.6 KB) - added by jummo4@… 6 years ago.
alpine-2.11_patch.diff (3.9 KB) - added by jummo4@… 6 years ago.
patch-alpine-Makefile.in.diff (842 bytes) - added by jerryyhom 5 years ago.
patch-alpine-2.11-alpine-Makefile.in.diff (866 bytes) - added by jerryyhom 5 years ago.
patch-yosemite-panic-rename.diff (21.2 KB) - added by jerryyhom 5 years ago.
Portfile-alpine.diff (1.8 KB) - added by jerryyhom 5 years ago.
Portfile-alpine-2.20.diff (1.7 KB) - added by jerryyhom 4 years ago.
updated checksums
Portfile-alpine-2.20.2.diff (1.6 KB) - added by jerryyhom 4 years ago.
plain sources

Download all attachments as: .zip

Change History (53)

Changed 6 years ago by jummo4@…

Attachment: alpine-2.10_patch.diff added

comment:1 Changed 6 years ago by jummo4@…

I have successfully installed Alpine 2.10 on my 10.8 system with the attached patch.

comment:2 Changed 6 years ago by macports.org@…

Alpine 2.11 has been released.

http://patches.freeiz.com/alpine/release/

Changed 6 years ago by jummo4@…

Attachment: alpine-2.11_patch.diff added

comment:3 Changed 6 years ago by jummo4@…

I have added a patch for Alpine 2.11.

comment:4 Changed 6 years ago by jummo4@…

Cc: jummo4@… added

Cc Me!

comment:5 Changed 5 years ago by patrick.sizun@…

Cc: patrick.sizun@… added

Cc Me!

comment:6 Changed 5 years ago by ryandesign (Ryan Schmidt)

Keywords: haspatch added
Owner: changed from larryv@… to ryandesign@…
Status: newassigned
Summary: alpine @2.00_4: update to 2.10alpine @2.00_4: update to 2.11

comment:7 Changed 5 years ago by ryandesign (Ryan Schmidt)

This does not build for me. I tested on 10.6, 10.7, 10.8, 10.9 and each time got an error like this:

Undefined symbols:
  "_libintl_setlocale", referenced from:
      _set_collation in libpithosd.a(collate.o)
ld: symbol(s) not found

comment:8 Changed 5 years ago by SickTeddyBear

Cc: amcgee@… added

Cc Me!

comment:9 Changed 5 years ago by jerryyhom

Hello! I would like to see the alpine port updated to 2.11. I grabbed the sources from Eduardo Chappa’s site, made a patch for the Portfile and updated an existing patch for the alpine sources, and have built it on 10.9. The patches should be attached.

I looked into and wanted to clean up the patches. The 2.11 sources seem to need only the patch for INTLLIBS which I updated. I also made the patch from the sources’ top-level directory, so patch.pre-args should no longer be needed. The osx-10.6 patch is already in the sources. I think the configure patch is now handled in the current configure script. The imap Makefile patch seems redundant since MacPorts already has ${prefix} and uses it in configure.args. I would appreciate any feedback.

I know the alpine port hasn’t had a maintainer in a while and John recently volunteered. I would also volunteer as co-maintainer if you don’t mind. I think the Portfile could be cleaned up a bit more, but for now I tried the minimum for building.

Last edited 5 years ago by jerryyhom (previous) (diff)

Changed 5 years ago by jerryyhom

comment:10 in reply to:  9 Changed 5 years ago by larryv (Lawrence Velázquez)

Cc: ryandesign@… added
Owner: changed from ryandesign@… to john@…
Status: assignednew

Replying to jerryyhom@…:

I know the alpine port hasn’t had a maintainer in a while and John recently volunteered. I would also volunteer as co-maintainer if you don’t mind.

If John is okay with it.

comment:11 Changed 5 years ago by jcvernaleo (John C. Vernaleo)

I'm totally fine with a co-maintainer.

As for the update, going to test build on my machine now and will replay back real soon.

comment:12 Changed 5 years ago by jcvernaleo (John C. Vernaleo)

jerryyhom@ couple minor issues with the Porfile you attached.

master_sites http://patches.freeiz.com/alpine/patches/alpine-2.11/alpine-2.11.tar.xz

That should not have the file name in it (and really should not have any version number in it). I think the correct path should be: master_sites http://patches.freeiz.com/alpine/release/src/

since that is more generic.

Also, the patch file alpine-2.00_panic_rename.patch is still needed for Yosemite.

Could you make those changes and let me know if it still builds okay on your system (10.9 I think you said).

Thanks!

comment:13 in reply to:  12 Changed 5 years ago by jerryyhom

Oops, I misunderstood the use of master_sites; simple fix, and thanks.

Re: the master_sites path, could you clarify what you mean by “more generic”? There are two source codes involved here.

I will work on the panic rename patch.

comment:14 Changed 5 years ago by jcvernaleo (John C. Vernaleo)

Sorry for not being clearer. By 'more generic' I just meant that it should not have the version in it, but you said you fixed that so don't worry about the 'more generic' comment.

Once you post the updated Portfile I'll test it out and hopefully we can get it in then.

comment:15 Changed 5 years ago by jerryyhom

Hm, I agree with you about going for more generic, except that in this case, Chappa created his path structure that way and his source codes all seem to be in different places. Anyhow, I thought you might have been talking about using plain 2.11 (which seems to be 2.00 with bug fixes) or 2.11 plus Chappa’s patches/features. Assuming the latter, I am attaching the Portfile patch along with the two accompanying patches. Thanks for testing.

Changed 5 years ago by jerryyhom

Changed 5 years ago by jerryyhom

comment:16 Changed 5 years ago by jcvernaleo (John C. Vernaleo)

What about:

master_sites http://patches.freeiz.com/alpine/patches/

distname ${name}-${version}/${name}-${version}

I don't like having the version number hardcoded into the master_sites variable since that isn't really what master sites is for. This way one can update this with less changes.

Other than that looks great to me.

comment:17 Changed 5 years ago by jerryyhom

Okay that works. I made one other change to make the yosemite patch apply only to yosemite. It’s a reminder to me that whenever 10.11 comes out, we’ll see if it was an anomaly or stable change on Apple’s part.

Changed 5 years ago by jerryyhom

Attachment: Portfile-alpine.diff added

comment:18 Changed 5 years ago by jcvernaleo (John C. Vernaleo)

Sorry for the slow reply. Busy weekend.

Based on past experience, that change is unlikely to be temporary, but I'm fine with leaving it as you have it.

So let's get these changes in.

comment:19 Changed 5 years ago by larryv (Lawrence Velázquez)

I’ll look at this soon-ish. There are a couple of changes that I’d like to add, to take advantage of the version bump.

comment:20 Changed 5 years ago by jcvernaleo (John C. Vernaleo)

larryv. Awesome, thanks!

comment:21 Changed 5 years ago by macports.org@…

Eduardo Chappa has now released 2.20.

comment:22 in reply to:  21 Changed 5 years ago by jcvernaleo (John C. Vernaleo)

Replying to macports.org@…:

Eduardo Chappa has now released 2.20.

While new version always excite me, I think we should ignore that until after we have 2.11 in.

comment:23 in reply to:  21 Changed 5 years ago by jerryyhom

Replying to macports.org@…:

Eduardo Chappa has now released 2.20.

Heh, great! Upstream bug fix simplifies our work by eliminating a patch... I should have just waited two weeks...

comment:24 Changed 5 years ago by jerryyhom

Among the several new features of alpine 2.20, one bonus which helps ports (including Homebrew and Fink) is the added configure support for testing alternate paths. The portfile could be simplified by eliminating about 25 lines, mostly configure options. The Makefile patch is now included upstream. Plus, in a nod to Yosemite (and maybe other systems), panic() has been renamed to alpine_panic() upstream. So alpine 2.20 should just build out of the box.

comment:25 in reply to:  24 ; Changed 5 years ago by larryv (Lawrence Velázquez)

Could you attach a patch for 2.20 then?

comment:26 Changed 5 years ago by jcvernaleo (John C. Vernaleo)

First thing tomorrow morning I'll update the patch for the Makefile.

comment:27 in reply to:  25 Changed 5 years ago by jerryyhom

Replying to larryv@…:

Could you attach a patch for 2.20 then?

I am attaching a portfile patch for 2.20 with minimum changes. The ssl related configure and build options are redundant and could be eliminated in the future. I think most of the other configure options are redundant too, but maybe there are historical reasons. I built it manually with no options against both openssl/libressl. Also, the 2.20 portfile should not need any patchfiles.

comment:28 Changed 5 years ago by jcvernaleo (John C. Vernaleo)

Looks good to me. larryv, I'm happy for that list diff to be committed and all the patches removed.

comment:29 Changed 5 years ago by jerryyhom

The checksum for the distfile may be changing while keeping the same filename because Chappa realized an important typo in his documentation. He has already updated his website docs, and I expect the distfile will update soon.

comment:30 Changed 4 years ago by jcvernaleo (John C. Vernaleo)

Changing files without changing versions is very very bad practice. I guess we can wait for that update to commit, but I really think we need to just go with what we have after that regardless of ANY other updates.

comment:31 Changed 4 years ago by jerryyhom

Yes, it is bad and just means more work for us. I am attaching the new portfile patch with updated checksums.

comment:32 Changed 4 years ago by jcvernaleo (John C. Vernaleo)

Great. larryv feel free to commit that (and delete the patches) when you get a chance.

Changed 4 years ago by jerryyhom

Attachment: Portfile-alpine-2.20.diff added

updated checksums

comment:33 Changed 4 years ago by larryv (Lawrence Velázquez)

It looks like you’re using the fully-patched source instead of the unpatched source. This is not workable because the distfile name does not change with the patchlevel. If upstream changes the patchlevel, users will start seeing checksum mismatches.

If we use the unpatched release, could we still remove all of our patches? Would your portfile patch still be valid (checksums aside)?

comment:34 Changed 4 years ago by jerryyhom

OK, I'll check the sourches without Chappa's patches. Thanks for the explanation.

I'm fairly certain the MacPorts patches can be removed because they deal with configuration issues and the panic() symbol clash which have been incorporated upstream.

comment:35 Changed 4 years ago by jerryyhom

I have built the plain sources and updated the portfile patch. Nothing new to report. Please have a try.

Changed 4 years ago by jerryyhom

Attachment: Portfile-alpine-2.20.2.diff added

plain sources

comment:36 Changed 4 years ago by larryv (Lawrence Velázquez)

Resolution: fixed
Status: newclosed

These should take care of everything. Let me know if I broke anything, especially with r133173.

comment:37 Changed 4 years ago by jerryyhom

Finally, alpine is available again to MP users; thanks Larry!

Re: [133173], considering I am new to MP, and I mean no disrespect, could you explain why transposing negative variants into positive ones is helpful here?

From a maintainer's perspective, feels like the changes introduce byzantine rules. At first, I thought it was simply double negatives to positive, but after hours of scratching my head, it seems more like triple negatives to keep the same build behavior? I am losing count, is it quadruple negatives? Overall, seems unnecessary and obfuscating.

comment:38 in reply to:  37 Changed 4 years ago by larryv (Lawrence Velázquez)

Replying to jerryyhom@…:

Re: [133173], considering I am new to MP, and I mean no disrespect, could you explain why transposing negative variants into positive ones is helpful here?

See the final paragraph of PortfileRecipes#default_variants. We want the act of enabling variants to enable features, not disable them. An implementation bug was the only reason negative variants were ever necessary. The vast majority of ports no longer have negative variants; alpine was just neglected until now. Today, the correct way to enable optional features by default is with the default_variants portfile option.

From a maintainer's perspective, feels like the changes introduce byzantine rules. At first, I thought it was simply double negatives to positive, but after hours of scratching my head, it seems more like triple negatives to keep the same build behavior? I am losing count, is it quadruple negatives? Overall, seems unnecessary and obfuscating.

It’s only the transitional code that’s confusing. The logic of positive variants is actually much less confusing than that of negative variants.

  • Since the without_tcl variant is deprecated, it should no longer be a default variant.
    • dports/mail/alpine/Portfile

      a b  
      3434    reinplace "s|__PREFIX__|${prefix}|" ${worksrcpath}/imap/Makefile
      3535}
      3636
      37 default_variants    +without_tcl
      38 
  • Without any variants selected, the port should have the minimal possible set of dependencies.
    • dports/mail/alpine/Portfile

      a b  
      39 depends_lib         port:openssl \
       37depends_lib         port:gettext \
      4038                    port:libiconv \
      41                     port:gettext \
      42                     port:openldap \
      43                     port:ncurses \
      44                     port:cyrus-sasl2
       39                    port:ncurses
  • Some build systems automatically enable optional features if they detect that the necessary dependencies are present. This is not acceptable to us, as it makes the builds irreproducible. The port needs to explicitly disable optional features if the variants that enable those optional features are not selected.
    • dports/mail/alpine/Portfile

      a b configure.args -with-lib-dir=${prefix}/lib \ 
      5348                    --with-ssl-include-dir=${prefix}/include/openssl \
      5449                    --with-ssl-lib-dir=${prefix}/lib \
      5550                    --with-local-password-cache-method \
      56                     --with-debug-level=0
       51                    --with-debug-level=0 \
       52                    --without-krb5 \
       53                    --without-ldap \
       54                    --without-ssl \
       55                    --without-tcl
      5756
      5857variant universal {}
      5958
      6059use_parallel_build  no
       60build.env           SSLTYPE=none
      6161build.args          CC=${configure.cc} \
      6262                    EXTRACFLAGS="[get_canonical_archflags cc]" \
      6363                    EXTRALDFLAGS="[get_canonical_archflags ld]" \
  • We turn the negative variants into dummy variants but keep them around for migration purposes. We usually give users about a year to upgrade their ports and switch over; then we remove the old stuff.
    • dports/mail/alpine/Portfile

      a b  
      6565
      66 variant without_krb5 description {Disable Kerberos5 support} {
      67     depends_lib-delete      port:cyrus-sasl2
      68     configure.args-append   --without-krb5
      69 }
      70 
      71 variant without_ldap description {Disable LDAP support} {
      72     depends_lib-delete      port:openldap
      73     configure.args-append   --without-ldap
      74 }
      75 
      76 variant without_ssl description {Disable SSL support} {
      77     depends_lib-delete      port:openssl
      78     configure.args-append   --without-ssl
      79     build.env-append        SSLTYPE=none
      80 }
      81 
      82 variant without_tcl description {Disable TCL support (disables Alpine Web)} {
      83     configure.args-append   --without-tcl
      84 }
       66# TODO: Remove legacy variants after 2016-02-20.
       67
       68variant without_krb5 conflicts kerberos description {Legacy variant} {}
       69variant without_ldap conflicts ldap description {Legacy variant} {}
       70variant without_ssl conflicts ssl description {Legacy variant} {}
       71variant without_tcl conflicts tcl description {Legacy variant} {}
      8572
  • Users who already have alpine installed need to be migrated to the new variants in a sane way. Ideally, we’d just set default_variants +kerberos +ldap +ssl and be done with it, but that would cause variant conflicts for users who have alpine installed with +without_krb5, +without_ldap, or +without_ssl. So we only set new default variants if the user doesn’t already have one of the old ones selected.

    I know this is confusing. Think about the first conditional. If a user has alpine +without_krb5 installed, we don’t want to set +kerberos as a default for them because they’ve already explicitly chosen to forego Kerberos support. So we only make +kerberos a default if they have not set the without_krb5 variant. The same goes for the rest. (We have to treat without_tcl / tcl a little differently because +without_tcl was the previous default.)

    The logical contortions one has to endure to reason about negative variants (“if {![variant_isset without_ldap]}”? what?) are actually a very good argument against their use. In any case, you need not worry about it, as it’s only transitional.

    • dports/mail/alpine/Portfile

      a b  
       73# TODO: Replace this morass after 2016-02-20.
       74#default_variants    +kerberos +ldap +ssl
       75
       76if {![variant_isset without_krb5]} {
       77    default_variants +kerberos
       78}
       79if {![variant_isset without_ldap]} {
       80    default_variants +ldap
       81}
       82if {![variant_isset without_ssl]} {
       83    default_variants +ssl
       84}
       85if {![variant_isset tcl]} {
       86    default_variants +without_tcl
       87}
       88if {![variant_isset without_tcl]} {
       89    default_variants +tcl
       90}
       91
      8692variant passfile description {Enable password files support} {
      8793    configure.args-delete   --with-local-password-cache-method
      8894    configure.args-append   --with-passfile=".pine.pwd"
      8995}
  • Finally, the positive variants modify the default port configuration to enable the features they control. Unfortunately, Alpine’s build system disables features using negative options, so we end up having to delete them to enable the features.
    • dports/mail/alpine/Portfile

      a b  
      8692variant passfile description {Enable password files support} {
      8793    configure.args-delete   --with-local-password-cache-method
      8894    configure.args-append   --with-passfile=".pine.pwd"
      8995}
       96
       97variant kerberos description {Kerberos support} {
       98    depends_lib-append      port:cyrus-sasl2
       99    configure.args-delete   --without-krb5
       100}
       101
       102variant ldap description {LDAP support} {
       103    depends_lib-append      port:openldap
       104    configure.args-delete   --without-ldap
       105}
       106
       107variant ssl description {OpenSSL support} {
       108    depends_lib-append      port:openssl
       109    configure.args-delete   --without-ssl
       110    build.env-delete        SSLTYPE=none
       111}
       112
       113variant tcl description {Tcl support (required by Alpine Web} {
       114    # Should we force MacPorts' Tcl?
       115    configure.args-delete   --without-tcl
       116}

comment:39 Changed 4 years ago by jerryyhom

OK, humor me while I try to understand your reply. Essentially, your considerable efforts are to transition the portfile to explicit positive variants while maintaining compatible behavior; after about a year, you will remove the transitional code, completing the transformation to explicit positive variants; everything is virtually identical. Please correct me if my summary is wrong.

Now, hopefully these are received as constructive comments. Please understand, my mindset is in trying to simplify and enhance alpine's portfile. On one hand, your changes seemed to go the other way and complicate it, but on the other hand, I think you could have saved yourself some time. Let me know if we should discuss this directly via email and off this ticket.

Generally, your explanation in your first paragraph probably belongs in the Guide. The portfile recipes and perhaps many parts of the wiki could go in the Guide.

Alpine specific, since you are cleaning up some depends_libs, I think all depends_libs may be removed. The libs iconv and ncurses are already provided by OS X; if not on older systems, they could be conditional, but probably a minor point here. Alpine builds on my system without gettext, but also probably a minor point. The openssl lib for the --without-ssl option (and variant) is tricky. Building that variant by itself will fail because the upstream code does not conditionally code around all parts. Adding --without-tcl will succeed. Probably no one has tried and compained, but if someone wanted the -ssl+tcl variants, that would fail.

When or why did the without_tcl variant become obsolete? Do you mean because MP implicitly has Tcl that the variant is obsolete? Your comment in the portfile is unclear.

Two more comments which tie together. Overall, more commenting would help, either in the portfile or descriptions about your changesets. You are experienced, know what you are doing, and if you were the only one looking at the code, you would easily go on doing fine work. However as you know, portfiles are under scrutiny by others, usually maintainers. Of course, you may do as you like as a committer, but as MP is a distributed management system, please be more mindful of others reading your code. I am trying to learn MP, but I am learning that the role of maintainer is easily trampled over by committers.

Last edited 4 years ago by jerryyhom (previous) (diff)

comment:40 in reply to:  39 Changed 4 years ago by larryv (Lawrence Velázquez)

Replying to jerryyhom@…:

OK, humor me while I try to understand your reply. Essentially, your considerable efforts are to transition the portfile to explicit positive variants while maintaining compatible behavior; after about a year, you will remove the transitional code, completing the transformation to explicit positive variants; everything is virtually identical. Please correct me if my summary is wrong.

Exactly right. It wasn’t that much effort; I’ve done this several times before.

Let me know if we should discuss this directly via email and off this ticket.

Yes, this is very much off-topic. I will start a thread on macports-dev.

comment:41 in reply to:  39 Changed 4 years ago by larryv (Lawrence Velázquez)

Replying to jerryyhom@…:

The openssl lib for the --without-ssl option (and variant) is tricky. Building that variant by itself will fail because the upstream code does not conditionally code around all parts. Adding --without-tcl will succeed. Probably no one has tried and compained, but if someone wanted the -ssl+tcl variants, that would fail.

I don’t follow. The --without-ssl configure option requires --without-tcl also?

comment:42 Changed 4 years ago by jerryyhom

I am guessing that not many people use the -ssl/+tcl variant and so is probably not worth worrying about. But, if someone complains, here's the issue. Compiling +tcl will try linking with ssl libs irrespective of -ssl. However, if suitable ssl libs are installed, compiling +tcl may succeed, irrespective of -ssl.

comment:43 in reply to:  42 Changed 4 years ago by larryv (Lawrence Velázquez)

Replying to jerryyhom@…:

I am guessing that not many people use the -ssl/+tcl variant and so is probably not worth worrying about. But, if someone complains, here's the issue. Compiling +tcl will try linking with ssl libs irrespective of -ssl. However, if suitable ssl libs are installed, compiling +tcl may succeed, irrespective of -ssl.

Okay, that’s not good. We should make +tcl require +ssl, then.

Does Alpine’s Tcl support actually use OpenSSL, or is it a bug? Has it been reported upstream?

comment:44 Changed 4 years ago by jerryyhom

I think it does use ssl and is not a bug, but I am just reporting from my compilation tests.

comment:45 Changed 4 years ago by larryv (Lawrence Velázquez)

There are actually further issues with the current port. I’ll open new tickets for them.

Note: See TracTickets for help on using tickets.