Opened 7 years ago

Closed 4 years ago

Last modified 4 years ago

#33259 closed enhancement (fixed)

cmake portgroup: out of source build option, possibly by default

Reported by: ryandesign (Ryan Schmidt) Owned by: larryv (Lawrence Velázquez)
Priority: Normal Milestone:
Component: ports Version:
Keywords: Cc: mojca (Mojca Miklavec), michaelld (Michael Dickens), stromnov (Andrew Stromnov), cooljeanius (Eric Gallager), RJVB (René Bertin), larryv (Lawrence Velázquez)
Port:

Description

Many of the ports that use the cmake portgroup manually do an out-of-source build. It would be nice if the cmake portgroup could do this on its own, possibly with a tbool option like "cmake.use_out_of_source_build" so that individual ports could opt-in.

Since so many of the ports using this portgroup do this, and out-of-source builds are a recommended practice, it might make sense to eventually make this option on by default. But all existing cmake portgroup ports would have to be tested to ensure they work with it on.

Change History (60)

comment:1 Changed 7 years ago by mojca (Mojca Miklavec)

Cc: mojca.miklavec.lists@… added

Cc Me!

comment:2 Changed 7 years ago by michaelld (Michael Dickens)

Cc: michaelld@… added

Cc Me!

comment:3 Changed 6 years ago by stromnov (Andrew Stromnov)

Cc: stromnov@… added

Cc Me!

comment:4 Changed 6 years ago by mojca (Mojca Miklavec)

Any news about this?

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

I have not been working on it, but still think it should be done.

comment:6 Changed 5 years ago by cooljeanius (Eric Gallager)

Cc: egall@… added

Cc Me!

comment:7 Changed 4 years ago by RJVB (René Bertin)

Cc: rjvbertin@… added

Cc Me!

comment:8 Changed 4 years ago by RJVB (René Bertin)

I agree.

The KDE4 PortGroup imposes this, in a way that appears to work just fine with all KDE4 ports. Maybe the only way to advance this (3yo!) ticket is to force the change and see what ports break? (It would be nice if MacPorts had a build slave that could do this testing and then send note to the maintainers of ports that get broken...)

comment:9 Changed 4 years ago by mojca (Mojca Miklavec)

Before dreaming about setting up new buildslaves, it would help to actually come up with a patch ;)

comment:10 Changed 4 years ago by mojca (Mojca Miklavec)

We could introduce a global variable like build_in_source yes/no or build_out_of_source yes/no that would default to in-source while the CMake PortGroup could set it to out-of-source by default. Individual ports could then opt in or out and the functionality wouldn't be limited to the CMake PortGroup alone.

comment:11 Changed 4 years ago by RJVB (René Bertin)

The point with that is: how are you going handle all different build systems? CMake has a portgroup exactly because it doesn't work the same as the legacy configure approach, ditto for qmake and the Python group. So even if base could support OOT builds for the systems it supports, the cmake (etc) portgroup files will still need to redefine the appropriate values.

You wanted a patch? Copying one inline:

diff --git a/trunk/dports/_resources/port1.0/group/cmake-1.0.tcl b/trunk/dports/_resources/port1.0/group/cmake-1.0.tcl
--- a/trunk/dports/_resources/port1.0/group/cmake-1.0.tcl       (revision 133751)
+++ b/trunk/dports/_resources/port1.0/group/cmake-1.0.tcl       (working copy)
@@ -134,3 +134,13 @@
 if {[string first "--enable-debug" ${configure.args}] > -1} {
     configure.args-delete     --enable-debug
 }
+
+# setup all ports using cmake to build in a separate directory from the source;
+# this setting must be the full directory path
+post-extract            { file mkdir ${workpath}/build }
+
+# standard post-arg, where to find the primary CMakeLists.txt file.
+default configure.post_args {../${worksrcdir}}
+default configure.dir       {${workpath}/build}
+default build.dir           {${workpath}/build}
+

That's copied over directly from the KDE4 1.1 portgroup, with just a slight change in the comment.

comment:12 in reply to:  11 Changed 4 years ago by mojca (Mojca Miklavec)

Replying to rjvbertin@…:

The point with that is: how are you going handle all different build systems?

I'm not yet sure about all cases.

It's not a big problem for ./configure. Setting the configure.dir and build.dir (as well as making one) should be the same. Of course one then needs to call ../<name>/configure instead of just ./configure. On the other hand autotools would probably have to be run in the same folder as the sources.

Of course that wouldn't cover all the cases and even cases with configure might fail for many packages simply because they are not properly written and don't support out-of-source builds properly. But leaving maintainers the option to opt in could be useful in some cases.

CMake has a portgroup exactly because it doesn't work the same as the legacy configure approach, ditto for qmake and the Python group. So even if base could support OOT builds for the systems it supports, the cmake (etc) portgroup files will still need to redefine the appropriate values.

Yes, sure. But we could have a conditional setting. If Portfile opts out of default behaviour, the PortGroup should not redefine the values for an out-of-source build.

You wanted a patch? Copying one inline:

diff --git a/trunk/dports/_resources/port1.0/group/cmake-1.0.tcl b/trunk/dports/_resources/port1.0/group/cmake-1.0.tcl
--- a/trunk/dports/_resources/port1.0/group/cmake-1.0.tcl       (revision 133751)
+++ b/trunk/dports/_resources/port1.0/group/cmake-1.0.tcl       (working copy)
@@ -134,3 +134,13 @@
 if {[string first "--enable-debug" ${configure.args}] > -1} {
     configure.args-delete     --enable-debug
 }
+
+# setup all ports using cmake to build in a separate directory from the source;
+# this setting must be the full directory path
+post-extract            { file mkdir ${workpath}/build }
+
+# standard post-arg, where to find the primary CMakeLists.txt file.
+default configure.post_args {../${worksrcdir}}
+default configure.dir       {${workpath}/build}
+default build.dir           {${workpath}/build}
+

It would probably help if we had some (very) basic configurability to opt out.

comment:13 Changed 4 years ago by michaelld (Michael Dickens)

My US$0.02 here: Let's do the global variable boolean optionsas suggested by Mojca that are off by default (for now). When a Portfile sets the value to on/true, we use the VPATH build. In this way, all current ports will still work and we can transition to using the new way as needed and time allows. At some time down the road, we can swap the default value to on/true, and update all of the ports using the CMake portgroup accordingly.

comment:14 Changed 4 years ago by RJVB (René Bertin)

OK...

This should do the trick, I think?

diff --git a/trunk/dports/_resources/port1.0/group/cmake-1.0.tcl b/trunk/dports/_resources/port1.0/group/cmake-1.0.tcl
--- a/trunk/dports/_resources/port1.0/group/cmake-1.0.tcl	(revision 133751)
+++ b/trunk/dports/_resources/port1.0/group/cmake-1.0.tcl	(working copy)
@@ -134,3 +134,17 @@
 if {[string first "--enable-debug" ${configure.args}] > -1} {
     configure.args-delete     --enable-debug
 }
+
+options cmake.build_out_of_source
+default cmake.build_out_of_source false
+
+if {cmake.build_out_of_source} {
+    # setup all ports using cmake to build in a separate directory from the source;
+    # this setting must be the full directory path
+    post-extract            { file mkdir ${workpath}/build }
+
+    # standard post-arg, where to find the primary CMakeLists.txt file.
+    default configure.post_args {../${worksrcdir}}
+    default configure.dir       {${workpath}/build}
+    default build.dir           {${workpath}/build}
+}

comment:15 Changed 4 years ago by mojca (Mojca Miklavec)

There's another aspect that I forgot about. Thanks for pointing it out. Once we do the necessary changes globally or in the PortGroup we could simply change the individual CMake-based ports to opt-in one-by-one. Then a single commit would just set the variable default to ON in the CMake PortGroup (and next commit would just remove all those opt-ins in CMake-based ports). Then we wouldn't have any pressure or fears of unexpected failures.

What's the best way to introduce a new global variable (if we can agree on that and its name)? Does one need to upgrade MacPorts for the change to take effect? (If we would need an upgrade, it might be better to first introduce in the CMake PortGroup nevertheless and then move it to a global place once a new release comes to the horizon.)

comment:16 in reply to:  15 Changed 4 years ago by michaelld (Michael Dickens)

Replying to mojca@…:

What's the best way to introduce a new global variable (if we can agree on that and its name)? Does one need to upgrade MacPorts for the change to take effect? (If we would need an upgrade, it might be better to first introduce in the CMake PortGroup nevertheless and then move it to a global place once a new release comes to the horizon.)

Good question. I'm not sure of the answer. Since the change is going into the PortGroup, I think it'll become part of user's install when they "selfupdate" or "sync", which means we can start using the variable immediately in Portfiles. I might be wrong though. Hopefully someone else knows for sure.

comment:17 Changed 4 years ago by mojca (Mojca Miklavec)

If we change just cmake-1.0.tcl, we don't need to wait for a new release. But if we would introduce a global variable (that could also be used for configure-based projects), we might need an upgrade of base (not sure about that).

Rene's last patch looks promising. I will test it on some of my ports.

comment:18 Changed 4 years ago by michaelld (Michael Dickens)

I think we can add the option to cmake-1.0.tcl, as Rene has done. I won't have time to test until tomorrow, so if your tests work Mojca I'd say go ahead and commit the patch.

comment:19 in reply to:  14 Changed 4 years ago by mojca (Mojca Miklavec)

Replying to rjvbertin@…:

+options cmake.build_out_of_source
+default cmake.build_out_of_source false
+
+if {cmake.build_out_of_source} {
+    # setup all ports using cmake to build in a separate directory from the source;
+    # this setting must be the full directory path
+    post-extract            { file mkdir ${workpath}/build }
+
+    # standard post-arg, where to find the primary CMakeLists.txt file.
+    default configure.post_args {../${worksrcdir}}
+    default configure.dir       {${workpath}/build}
+    default build.dir           {${workpath}/build}
+}

What is the role of the "default" keyword in "default build.dir" for example? Couldn't it be left out?

comment:20 Changed 4 years ago by mojca (Mojca Miklavec)

I did some very very basic testing, but I couldn't get the code to work in the way it was written even after removing the typos and obvious problems. I suspect the problem is that

if {${cmake.build_out_of_source}} {

is only evaluated once. And the setting

cmake.build_out_of_source true

after importing the PortGroup has no effect whatsoever. I assume that this should be a procedure rather than a setting.

Here's a draft that worked for me with at least one port:

options cmake.build_out_of_source
option_proc cmake.build_out_of_source cmake._set_out_of_source_build

proc cmake._set_out_of_source_build {option action args} {
    global workpath worksrcpath configure.dir build.dir

    if {"set" ne ${action}} {
        return
    }

    if {${args} eq "true"} {
        configure.dir       ${workpath}/build
        build.dir           ${configure.dir}
        # standard post-arg, where the primary CMakeLists.txt file can be found
        configure.post_args ${worksrcpath}
        # create the build directory
        post-extract        { file mkdir ${build.dir} }
    }
}

I was copy-pasting from another PortGroup, so I'm not sure if I actually got all the bits and pieces right.

The above code doesn't allow overwriting configure.dir for example. That's probably bad.

Last edited 4 years ago by mojca (Mojca Miklavec) (previous) (diff)

comment:21 Changed 4 years ago by neverpanic (Clemens Lang)

I'd rather shorten the option name to cmake.out_of_source and use yes and no over true and false like we usually do. Your code only allows true, which is bad, because maintainers will try to set it to yes or 1.

Also, I'd rather modify the defaults than change the actual values when cmake.out_of_source yes is encountered in a Portfile. That would make

configure.dir ${worksrcpath}/foo
cmake.out_of_source yes

work correctly. It's not obvious which side effects assigning to out_of_source has, otherwise.

comment:22 Changed 4 years ago by neverpanic (Clemens Lang)

Here's a patch:

  • cmake-1.0.tcl

     
    134134if {[string first "--enable-debug" ${configure.args}] > -1} {
    135135    configure.args-delete     --enable-debug
    136136}
     137
     138options cmake.out_of_source
     139default cmake.out_of_source yes
     140
     141options cmake.build_dir
     142default cmake.build_dir {${workpath}/build}
     143
     144proc _cmake_get_build_dir {} {
     145    global cmake.out_of_source cmake.build_dir
     146
     147    if {${cmake.out_of_source}} {
     148        return ${cmake.build_dir}
     149    } else {
     150        return ${worksrcpath}
     151    }
     152}
     153
     154pre-configure {
     155    file mkdir ${configure.dir}
     156}
     157
     158default configure.dir       {[_cmake_get_build_dir]}
     159default configure.post_args {../${worksrcdir}}
     160default build.dir           {${configure.dir}}

Tested and works against weechat. I've left the default at yes, that needs to be changed if you want to test every port manually. Also, the weechat port seems to need network access to build manpages.

comment:23 in reply to:  21 Changed 4 years ago by mojca (Mojca Miklavec)

Replying to cal@…:

I'd rather shorten the option name to cmake.out_of_source

OK.

and use yes and no over true and false like we usually do. Your code only allows true, which is bad, because maintainers will try to set it to yes or 1.

That was just bad coding that should be fixed of course.

Also, I'd rather modify the defaults than change the actual values when cmake.out_of_source yes is encountered in a Portfile. That would make

configure.dir ${worksrcpath}/foo
cmake.out_of_source yes

work correctly.

Ah, thank you. That answers my previous question about the role of default.

I'm a bit sceptic about the following:

default configure.post_args {../${worksrcdir}}

Setting something like

configure.dir ${workpath}/build/i386

would probably no longer resolve "../${worksrcdir}" properly. I believe that using ${worksrcpath} is safer from that respect.

comment:24 Changed 4 years ago by RJVB (René Bertin)

The role of default would be to set a default value that is applied when the variable hasn't been set already, no? That was how I saw its use in any case, I guess I should have stated that the OOT had to be set before including the port group (did you test that?).

As to your last concern: I think it's not possible to cope with all possible f-ups prophylactically. Someone who sets configure.dir to a non-standard value should be prepared to change configure.post_args accordingly, IMHO.

BTW, here's an extract from the qt5-mac Portfile, for the sqlite plugin (which uses qmake instead of cmake, and doesn't do an OOT build):

    # for single architecture, easier to use
    #    worksrcdir ${worksrcdir}/qtbase/src/plugins/sqldrivers/sqlite,
    #    but doesn't work for universal build
    configure.dir ${worksrcpath}/qtbase/src/plugins/sqldrivers/sqlite
    build.dir     ${configure.dir}
    destroot.dir  ${configure.dir}
Last edited 4 years ago by RJVB (René Bertin) (previous) (diff)

comment:25 in reply to:  24 ; Changed 4 years ago by ryandesign (Ryan Schmidt)

Replying to mojca@…:

Setting something like

configure.dir ${workpath}/build/i386

would probably no longer resolve "../${worksrcdir}" properly. I believe that using ${worksrcpath} is safer from that respect.

Yes, you're right.

Replying to rjvbertin@…:

The role of default would be to set a default value that is applied when the variable hasn't been set already, no? That was how I saw its use in any case,

Yes, that's correct. It sets a default, which ports can still override if needed. Notably, if you write default someoption {somevalue}, then it does not matter if the portfile sets someoption to another value before or after including the portgroup; it'll work either way (the portfile's value will be used and the default will not). Whereas if you wrote someoption somevalue in a portgroup, if a portfile set the value before including the portgroup, the portgroup's value would override the portfile's.

I guess I should have stated that the OOT had to be set before including the port group (did you test that?).

Portgroups should not be written to require that.

comment:26 in reply to:  25 Changed 4 years ago by mojca (Mojca Miklavec)

Replying to ryandesign@…:

Replying to mojca@…:

I guess I should have stated that the OOT had to be set before including the port group (did you test that?).

Portgroups should not be written to require that.

No, I didn't test that, but I agree with Ryan here.

comment:27 Changed 4 years ago by RJVB (René Bertin)

Hmmm: https://guide.macports.org/#development.obsolete-portgroup

Note
It is important to specify replaced_by BEFORE the PortGroup line!

O:-)

comment:28 Changed 4 years ago by mojca (Mojca Miklavec)

Yes, I wanted to point out that exception as well. Maybe we just need to open a new ticket.

comment:29 Changed 4 years ago by ryandesign (Ryan Schmidt)

Yes, ideally the obsolete 1.0 portgroup shouldn't have been written that way either. I think the reason is that it was desired to include the name of the replacement port in the description, and the default keyword did not work together with the description keyword. (Possibly, default doesn't work with keywords that get included in the portindex.)

comment:30 Changed 4 years ago by neverpanic (Clemens Lang)

So, summing up, the above patch with ${worksrcpath} instead of ../${worksrcdir} should work in virtually all cases. Should we commit it?

comment:31 Changed 4 years ago by mojca (Mojca Miklavec)

I support committing it, along with a short notice to all maintainers responsible for Portfiles using the CMake PortGroup.

The only thing I'm not sure about is whether the default for the option should be yes or no. We can do a bold move and then quickly fix whatever port breaks. Or we could be on the safe side, but then have a lot more work to do (setting the value to yes in every port, switch the default once all the ports are tested, then remove the option from all the ports again). Perhaps being a bit more adventurous would pay off and introduce less work.

comment:32 Changed 4 years ago by michaelld (Michael Dickens)

I have a bunch of ports that already do VPATH building (as this type of building is known to me as). I'd prefer to be able to move these over as I get to them, as opposed to being forced to fix them all ASAP -- so, my vote is to set the option to "no" by default. But, if enough others want to do "yes" then I'll figure out a way to make it work. I think the amount of "work" required by us is about the same, it's the question of whether that work is required all up front (ASAP) or spread out over time.

comment:33 Changed 4 years ago by mojca (Mojca Miklavec)

I'm not familiar with VPATH, but you could always list those ports and someone would just add "cmake.out_of_source no" to all of them. Hopefully without the need for any other fix.

comment:34 Changed 4 years ago by michaelld (Michael Dickens)

I "grew up" using GNU Autotools. VPATH == "out of source tree build". See also: http://www.gnu.org/software/automake/manual/html_node/VPATH-Builds.html

comment:35 Changed 4 years ago by mojca (Mojca Miklavec)

OK. I searched for the term, but misunderstood other explanations.

I tried some of my ports that already use out-of-source builds and they kept working without modifications after changing the PortGroup. (On the other hand that is bad: we could then easily ignore the changes or or forget reverting back the existing hacks.)

comment:36 Changed 4 years ago by michaelld (Michael Dickens)

Would someone post the "current final" patch so that I can try it out?

comment:37 Changed 4 years ago by RJVB (René Bertin)

The patch is in comment 22, though it claims to be w.r.t a revision 133760 while according to my svn cmake-1.0.tcl is still at rev. 129400 ...

comment:38 Changed 4 years ago by RJVB (René Bertin)

And shouldn't the default configure.post_args be set the same way the default ${configure.dir} is set, i.e. as a function of ${cmake.out_of_source}? (= left unspecified when !${cmake.out_of_source}?)

Last edited 4 years ago by RJVB (René Bertin) (previous) (diff)

comment:39 in reply to:  36 Changed 4 years ago by mojca (Mojca Miklavec)

Replying to michaelld@…:

Would someone post the "current final" patch so that I can try it out?

I used cal's patch with the mentioned minor modification for configure.post_args (and using yes/no as the default value is still debatable):

  • cmake-1.0.tcl

     
    134134if {[string first "--enable-debug" ${configure.args}] > -1} {
    135135    configure.args-delete     --enable-debug
    136136}
     137
     138options cmake.out_of_source
     139default cmake.out_of_source yes
     140
     141options cmake.build_dir
     142default cmake.build_dir {${workpath}/build}
     143
     144proc _cmake_get_build_dir {} {
     145    global cmake.out_of_source cmake.build_dir worksrcpath
     146
     147    if {${cmake.out_of_source}} {
     148        return ${cmake.build_dir}
     149    } else {
     150        return ${worksrcpath}
     151    }
     152}
     153
     154pre-configure {
     155    file mkdir ${configure.dir}
     156}
     157
     158default configure.dir       {[_cmake_get_build_dir]}
     159default configure.post_args {${worksrcpath}}
     160default build.dir           {${configure.dir}}

Replying to rjvbertin@…:

The patch is in comment 22, though it claims to be w.r.t a revision 133760 while according to my svn cmake-1.0.tcl is still at rev. 129400 ...

The patch was probably a result of "svn diff" which gives you the revision of the checkout, at least in my case. I wouldn't bother with the number being too high.

And shouldn't the default configure.post_args be set the same way the default ${configure.dir} is set, i.e. as a function of ${cmake.out_of_source}? (= left unspecified when !${cmake.out_of_source}?)

I don't see what could go wrong if the value is always specified. The value should be correct even when building in the same tree where the sources are.

Last edited 4 years ago by mojca (Mojca Miklavec) (previous) (diff)

comment:40 Changed 4 years ago by RJVB (René Bertin)

I tried with one of my ports. It worked fine with the default setting as shown in comment 22, i.e. OOT builds by default. But I got errors about undefined variables when I set the default to no in the portgroup file.

comment:41 Changed 4 years ago by RJVB (René Bertin)

That issue remains with the patch in comment 39. When I set cmake.out_of_source to no in the resulting cmake-1.0.tcl file, and then try to configure a port that uses this portgroup, I get

--->  Configuring QtCurve-qt5
Error: org.macports.configure for port QtCurve-qt5 returned: can't read "configure.dir": can't read "worksrcpath": no such variable

I get the same error when I set cmake.out_of_source to no in the port's Portfile, after including the cmake portgroup.

BTW: apparently one will need to set cmake.out_of_tree no when setting the property *before* including the cmake portgroup?

Last edited 4 years ago by RJVB (René Bertin) (previous) (diff)

comment:42 Changed 4 years ago by mojca (Mojca Miklavec)

Tiny error. Just add worksrcpath to global variables and then it should work (I fixed comment:39 as well).

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

Cc: larryv@… added

Cc Me!

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

Replying to mojca@…:

The only thing I'm not sure about is whether the default for the option should be yes or no. We can do a bold move and then quickly fix whatever port breaks.

I’m with Michael on this. We should not risk breaking ports intentionally.

Or we could be on the safe side, but then have a lot more work to do (setting the value to yes in every port, switch the default once all the ports are tested, then remove the option from all the ports again).

Or we could actually make use of the portgroup version number. With a cmake-1.1 portgroup that enables out-of-source builds by default, we’d still have to migrate ports one at a time, but we’d need just one pass to update the PortGroup command and remove obsolete workarounds. I have this set up locally, and it seems to work fine. I’d like to commit it this week if no one has any objections to this migration strategy.

comment:45 Changed 4 years ago by michaelld (Michael Dickens)

I tested the latest version on a few ports, and they all worked. I don't like the multiple use of "mkdir" even when it is unnecessary (not out of tree build), but it works (I think 'port' internally either checks to see if the directory already exists & if so then does nothing, or just ignores what the system call returns; I didn't test beyond this). That said, I would prefer to not break anything up front, so allow ports to opt in as they can. Somewhere down the road we do a swap-around & fix whatever is left. This could be done via a cmake-1.1 PortGroup, yes; that might be the better way to do this, since it would require no changes down the road. I vote for going ahead and committing. I'll probably tweak the mkdir stuff to make me happy :)

Last edited 4 years ago by michaelld (Michael Dickens) (previous) (diff)

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

Replying to michaelld@…:

I don't like the multiple use of "mkdir" even when it is unnecessary (not out of tree build), but it works (I think 'port' internally either checks to see if the directory already exists & if so then does nothing, or just ignores what the system call returns; I didn't test beyond this).

Tcl’s file mkdir command does nothing if the directory already exists. I think it’s nice to not have to check first :)

http://www.tcl.tk/man/tcl8.5/TclCmd/file.htm#M22

(Incidentally, I don’t see why base doesn’t create configure.dir automatically, but that’s a question for another time.)

comment:47 Changed 4 years ago by michaelld (Michael Dickens)

Cool. That makes me happy without even putting the check in place!

I think the only downside to using a cmake-1.1 is that any non-OOS changes to it also need to go into cmake-1.0. So, some duplication that would otherwise not be necessary. Still, it seems like a small penalty to pay to get this functionality in place without changes down the road. Probably best to add a note to both PortGroups to this effect (changing one requires changing the other in the same way).

comment:48 Changed 4 years ago by RJVB (René Bertin)

Is there or has there ever been a cmake-0.0 portgroup? I'd feel more comfortable moving the old default behaviour to a "previous version" rather than obliging everyone to delve into their portfiles or portgroup files to bump a version when their products just work with the new OOS approach. It also means that maintainers who do nothing will keep using non-OOS builds, which kind of goes against the very reasons OOS builds were made the default ... Note that demoting the current default to a legacy portgroup also means there is no or less reason to port other modifications to it...

comment:49 in reply to:  48 ; Changed 4 years ago by larryv (Lawrence Velázquez)

Replying to rjvbertin@…:

Is there or has there ever been a cmake-0.0 portgroup? I'd feel more comfortable moving the old default behaviour to a "previous version"

What? This would be weird and confusing.

rather than obliging everyone to delve into their portfiles or portgroup files to bump a version when their products just work with the new OOS approach. It also means that maintainers who do nothing will keep using non-OOS builds

Yes, that’s what Michael and I want because we don’t know that all the portfiles will “just work”. So the question is whether this should be opt-in or opt-out.

If we decided on opt-out, we already have a simple implementation: Change the default in cmake-1.0 and be done with it. Ports that can’t deal with the default behavior would just add “cmake.out_of_source no”.

which kind of goes against the very reasons OOS builds were made the default

I don’t agree. The point of portgroups is to cut down on portfile boilerplate, not necessarily to reduce maintainer effort.

Also: Given that this modification would change the default build behavior, I think it’d be a good thing to require portfiles to be updated for it. It’s confusing when a port’s behavior mysteriously changes because of portgroup action-at-a-distance, especially if it breaks. This happens all too often, and it can be difficult to debug.

Note that demoting the current default to a legacy portgroup also means there is no or less reason to port other modifications to it...

I think maintaining parallel portgroups is a minor problem. Forgetting to change one or the other would not be the end of the world; someone else could just clean up afterwards.

comment:50 in reply to:  49 ; Changed 4 years ago by RJVB (René Bertin)

Replying to larryv@…:

which kind of goes against the very reasons OOS builds were made the default

I don’t agree. The point of portgroups is to cut down on portfile boilerplate, not necessarily to reduce maintainer effort.

I wasn't talking about the reason to use a portgroup, but to make OOS builds the default in the cmake portgroup.

Also: Given that this modification would change the default build behavior, I think it’d be a good thing to require portfiles to be updated for it. It’s confusing when a port’s behavior mysteriously changes because of portgroup action-at-a-distance, especially if it breaks. This happens all too often, and it can be difficult to debug.

I this case it should be sufficiently clear from looking (ok, squinting hard) at the paths shown in the main.log Portfiles that just do a standard cmake - build - install shouldn't run into any problems, because cmake itself has no issues doing OOS builds.

Anyway, with GSoC coming up, why not tinker the portgroup inclusion logic a bit? A change as proposed would fit in very well, I think, with a scheme in which Portgroup cmake 1 loads the most recent v1.x portgroup, while PortGroup cmake 1.x would include the exact 1.x version. That would allow portfiles that don't do anything "weird" like copying things from ${build.dir} to ${destroot} can always use the latest portgroup version, while ports with specific requirements can specify what version they need. This seems so logical when versions are already supported that the feature maybe exists already?

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

Replying to rjvbertin@…:

I wasn't talking about the reason to use a portgroup, but to make OOS builds the default in the cmake portgroup.

I don’t see that this is any different. The point here is not necessarily to force all CMake ports to use out-of-source builds; it’s to eliminate the boilerplate that is currently required to do so.

I this case it should be sufficiently clear from looking (ok, squinting hard) at the paths shown in the main.log Portfiles that just do a standard cmake - build - install shouldn't run into any problems, because cmake itself has no issues doing OOS builds.

Sure, but I still don’t think that magically changing the build behavior from the other end of the repository (so to speak) is good practice.

Anyway, with GSoC coming up, why not tinker the portgroup inclusion logic a bit? A change as proposed would fit in very well, I think, with a scheme in which Portgroup cmake 1 loads the most recent v1.x portgroup, while PortGroup cmake 1.x would include the exact 1.x version. That would allow portfiles that don't do anything "weird" like copying things from ${build.dir} to ${destroot} can always use the latest portgroup version, while ports with specific requirements can specify what version they need. This seems so logical when versions are already supported that the feature maybe exists already?

It doesn’t; the version is used as-is. Better versioning is not a bad idea, but discuss it elsewhere. It’s a policy matter, not a technical one. (Implementing this would take literally ten minutes.)

comment:52 Changed 4 years ago by mojca (Mojca Miklavec)

Replying to larryv@…:

Or we could actually make use of the portgroup version number. With a cmake-1.1 portgroup that enables out-of-source builds by default, we’d still have to migrate ports one at a time, but we’d need just one pass to update the PortGroup command and remove obsolete workarounds. I have this set up locally, and it seems to work fine. I’d like to commit it this week if no one has any objections to this migration strategy.

I'm not really a fan of increasing the portgroup version number. We would have to increase the number if we introduced some incompatible change. Otherwise we just end up with two files (both of which need to be maintained) with hardly any functional difference.

If most developers have a preference to keep the old functionality by default, let's just use "cmake.out_of_source no" for now and start changing the ports one-by-one. Once all the ports have been adapted and tested, just switch the default and remove "cmake.out_of_source yes" from all the ports. The last step sounds straightforward and can be fully automated.

I really don't think that advantages of increasing the PortGroup number outweight the extra "troubles".

comment:53 Changed 4 years ago by mojca (Mojca Miklavec)

And if people are worried about file mkdir, we can certainly change the PortGroup in such a way that there will be absolutely zero side effects unless one specifies "cmake.out_of_source yes".

comment:54 Changed 4 years ago by michaelld (Michael Dickens)

I'm OK with "file mkdir" now; I was concerned that TCL was hiding something. No problems.

I you feel strongly, Mojca, about not moving to cmake-1.1, then I'm OK with integrating into cmake-1.0 & setting the default to "no". I think this is wiser than doing "yes" up front.

comment:55 Changed 4 years ago by mojca (Mojca Miklavec)

Yes, I would really really prefer to put the functionality into cmake-1.0.

Any other objections to the above mentioned patch other than replacing yes by no?

Any volunteer to commit the change?

The ticket is more than three years old, it would be about time to close it.

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

Owner: changed from macports-tickets@… to larryv@…
Status: newassigned

Replying to mojca@…:

Yes, I would really really prefer to put the functionality into cmake-1.0.

Any other objections to the above mentioned patch other than replacing yes by no?

I’m fine with this. The important thing to me is that the behavior is opt-in.

Any volunteer to commit the change?

I’ll do it.

comment:57 Changed 4 years ago by michaelld (Michael Dickens)

All works for me. Make it so, Larryv.

comment:58 Changed 4 years ago by neverpanic (Clemens Lang)

http://i.imgur.com/8a18WSP.jpg

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

Resolution: fixed
Status: assignedclosed

ENGAGE

r134128

comment:60 Changed 4 years ago by mojca (Mojca Miklavec)

Great, thank you. I opened a new ticket to inform the maintainers, #47197.

Now let's add another option to disable addition of CMAKE_OSX_ARCHITECTURES, ticket #37732. Thanks in advance to everyone.

Note: See TracTickets for help on using tickets.