weechat: python variants

[ryandesign (Ryan Carsten Schmidt)]

The weechat port has a python variant:

variant python description {Bindings for python plugins} {
    configure.args-delete   --disable-python
    configure.args-append   --enable-python
    depends_lib-append      path:bin/python:python26
}

The dependency on path:bin/python:python26 is incorrect because the python26 port does not provide /opt/local/bin/python; it provides /opt/local/bin/python2.6. You should offer variants for each supported version of python (e.g. python25, python26, python27) and in each one depend on the correct python port and use configure args, environment variables or patches to communicate to the weechat build system which version of python should be used. The python variant should be kept around for one year, turned into a stub variant, depending on the python26 variant, to help users upgrade. ("variant python requires python26 description {Legacy compatibility variant} {}")

Or if you're feeling adventurous, and even better solution would be to offer the python bindings as subports instead of as variants. Not sure it the weechat build system will easily accommodate that however.

[neverpanic (Clemens Lang)]

Re-assigning to new maintainer.

[kurthindenburg (Kurt Hindenburg)]

Cc Me!

[kurthindenburg (Kurt Hindenburg)]

starkhalo, do the variants build OK for you? I'm getting errors on +python27

clang: error: no such file or directory: 'Python.framework/Versions/2.7/Python'
make[2]: *** [src/plugins/python/python.so] Error 2

[harciga]

Sure they did, before the cmake change.

 cmake.out_of_source yes

Guess I'll fix the $patchfile later, I have to see in the PortGroup cmake what that does to ${worksrcpath}

Cheers.

starkhalo, do the variants build OK for you? I'm getting errors on +python27

clang: error: no such file or directory: 'Python.framework/Versions/2.7/Python'
make[2]: *** [src/plugins/python/python.so] Error 2

[harciga]

This appears to do the trick

diff --git a/ongoing b/Portfile
index 7336489..50095ef 100644
--- a/Portfile.orig
+++ b/Portfile
@@ -95,7 +95,7 @@ foreach s ${pythons_suffixes} {
 
 # TODO: Ideally this should go inside its corresponding python variant.
 post-configure {
-    set patchfile ${worksrcpath}/src/plugins/python/CMakeFiles/python.dir/link.txt
+    set patchfile ${worksrcpath}/../build/src/plugins/python/CMakeFiles/python.dir/link.txt
 
     if {[file exists ${patchfile}]} {
         reinplace -E "s| \(Python.framework\)| ${frameworks_dir}/\\1|g" ${patchfile}

I'll verify later tonight with the subport and py34/py35

[larryv (Lawrence Velázquez)]

Do not assume that the CMake build directory is “$worksrcpath/../build”. Use “${cmake.build_dir}”.

[larryv (Lawrence Velázquez)]

Actually, that would assume cmake.out_of_source yes. You should use ${configure.dir}.

[harciga]

Replying to larryv@…:

Actually, that would assume cmake.out_of_source yes. You should use ${configure.dir}.

Thanks, I'll update the ${patchfile} with ${configure.dir} and verify that it all builds fine later tonight.

[Ionic (Mihai Moldovan)]

Two nits:

1. For python3X, you never delete -DENABLE_PYTHON=OFF. Given that -DENABLE_PYTHON=ON is added afterwards, let's hope cmake always uses the last seen option to override previous ones? For python2X, -DENABLE_PYTHON=ON is passed two times (should cause no harm functionality-wise.)

2. depends_lib-append path:bin/python:${p} should be depends_lib-append port:${p}. bin/python is not provided by any port but set via port select (if the user even did that - there's enough installations with no "default" python binary selected) and you have no way of telling whether this binary corresponds to the selected python variant because you don't know the version.

[Ionic (Mihai Moldovan)]

I've said this on IRC before, but you probably missed it:

You don't want to check if the last digit/character is a three, but the first one. So the comment should be updated and the regex changed from (3$) to (^3).

Everything else looks good to me.

[larryv (Lawrence Velázquez)]

If you’re going to go with the regex, this…

set t ""

…is unnecessary. You just need this.

regexp {(^3)?} $s t

If s does not begin with “3”, then t will be empty.

[larryv (Lawrence Velázquez)]

adding variants without eval+subst

[larryv (Lawrence Velázquez)]

To be honest, I don’t think there’s any need to dynamically generate the variants like this. There shouldn’t be more than 4 Python variants ever (by policy), and it makes the portfile much harder for the rest of us to understand. The patch I just added demonstrates how most of the common logic can be abstracted out in a different way. (I haven’t tested it.)

[Ionic (Mihai Moldovan)]

Please use notes instead of ui_warn.

Are you sure you don't want to handle common variants code like larry suggested?

[larryv (Lawrence Velázquez)]

You’re duplicating a lot of code here. I strongly suggest doing it my way.

Also drop the 3.5 variant. We aren’t adding any 3.5 stuff until the stable release.

[harciga]

Replying to ionic@…:

Please use notes instead of ui_warn.

Are you sure you don't want to handle common variants code like larry suggested?

Replaced ui_warn with `notes. No.

[harciga]

Replying to larryv@…:

You’re duplicating a lot of code here. I strongly suggest doing it my way.

Also drop the 3.5 variant. We aren’t adding any 3.5 stuff until the stable release.

Dropped python35 until further notice.

[Ionic (Mihai Moldovan)]

Ok.

No, you aren't sure, or no, you won't change it? I'm fine with either, if you want me to commit it, please say it.

[harciga]

Replying to ionic@…:

Ok.

No, you aren't sure, or no, you won't change it? I'm fine with either, if you want me to commit it, please say it.

I don't see the need to change it again, the version I rewrote is clearer and after removing the python35 variant, just a few lines longer.

Looks to me it's just a question of style at this point, and since it works fine either way, just commit it as is.

Please.

[Ionic (Mihai Moldovan)]

Committed as r138277. Thanks!