Opened 8 years ago

Closed 8 years ago

#51643 closed defect (fixed)

qt5 portgroup overwrites supported_archs, defines universal variant when one is not desired

Reported by: ryandesign (Ryan Carsten Schmidt) Owned by: MarcusCalhoun-Lopez (Marcus Calhoun-Lopez)
Priority: Normal Milestone:
Component: ports Version: 2.3.4
Keywords: Cc: mkae (Marko Käning)
Port: qt5

Description

I'm trying to update my QupZilla port to version 2. It now requires qt5 instead of qt4. It also now requires an x86_64 processor.

Without using any portgroup, setting supported_archs x86_64 in a portfile causes the universal variant to be disabled. That's what I want. But something different happens when using the qt5 portgroup:

  • If I set supported_archs x86_64 before including the qmake5 portgroup, the qmake5 portgroup includes the qt5 portgroup which overwrites my supported_archs choice.
  • If I set supported_archs x86_64 after including the qmake5 portgroup, the qmake5 portgroup includes the qt5 portgroup which includes the muniversal portgroup which defines a universal variant.

It looks like the only way to make a working portfile with the way the qt5 portgroup is written today is to set universal_variant no before including the qmake5 portgroup and to set supported_archs x86_64 after including the qmake5 portgroup. That's inelegant.

It would be simple enough to modify the portgroup to set a default for supported_archs but not overwrite a value if the port already set one. Ideally, the portfile author should be able to specify supported_archs or universal_variant no later in the portfile, as is customary, but I'm not sure how the portgroup would then be able to set up the universal variant correctly for those ports that need it.

The app portgroup handles a similar situation (whether or not to add the makeicns dependency) using variable traces. That's a bit messy, but I can't think of another way to handle it.

Attachments (1)

patch-qt5-1.0.tcl.diff (1.7 KB) - added by MarcusCalhoun-Lopez (Marcus Calhoun-Lopez) 8 years ago.

Download all attachments as: .zip

Change History (9)

comment:1 Changed 8 years ago by MarcusCalhoun-Lopez (Marcus Calhoun-Lopez)

Status: newassigned

I have not tested this thoroughly, but how about something like this:
In qt5-1.0.tcl, replace

# no universal binary support in Qt 5                                                                                                                                                                       
#     see http://lists.qt-project.org/pipermail/interest/2012-December/005038.html                                                                                                                          
#     and https://bugreports.qt.io/browse/QTBUG-24952                                                                                                                                                       
supported_archs i386 x86_64
if { ![exists universal_variant] || [option universal_variant] } {
    PortGroup muniversal 1.0
    universal_archs_supported i386 x86_64
}

with

# no universal binary support in Qt 5                                                                                                                                                                       
#     see http://lists.qt-project.org/pipermail/interest/2012-December/005038.html                                                                                                                          
#     and https://bugreports.qt.io/browse/QTBUG-24952                                                                                                                                                       
default supported_archs {"i386 x86_64"}
# override universal_setup found in portutil.tcl so it uses muniversal PortGroup                                                                                                                            
# see #51643                                                                                                                                                                                                
proc universal_setup {args} {
    if {[variant_exists universal]} {
        ui_debug "universal variant already exists, so not adding the default one"
    } elseif {[exists universal_variant] && ![option universal_variant]} {
        ui_debug "universal_variant is false, so not adding the default universal variant"
    } elseif {[exists use_xmkmf] && [option use_xmkmf]} {
        ui_debug "using xmkmf, so not adding the default universal variant"
    } elseif {![exists os.universal_supported] || ![option os.universal_supported]} {
        ui_debug "OS doesn't support universal builds, so not adding the default universal variant"
    } elseif {[llength [option supported_archs]] == 1} {
        ui_debug "only one arch supported, so not adding the default universal variant"
    } else {
	ui_debug "adding universal variant via PortGroup muniversal"
        uplevel "PortGroup muniversal 1.0"
	uplevel "default universal_archs_supported {\"i386 x86_64\"}"
    }
}

Changed 8 years ago by MarcusCalhoun-Lopez (Marcus Calhoun-Lopez)

Attachment: patch-qt5-1.0.tcl.diff added

comment:2 Changed 8 years ago by mkae (Marko Käning)

Cc: mk@… added

Cc Me!

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

Has anyone tested the muniversal build lately, or a 32bit build? Last time I did was with Qt 5.4.1 .

Is there a point in checking the xmkmf option? That's an (old) make file generator for X11 applications, I doubt we'll encounter it with Qt5 software...

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

qt5 portgroup which includes the muniversal portgroup which defines a universal variant.

This is always the case because a universal Qt5 build can only be generated that way. Should one avoid loading it multiple times?

Regardless of that, wouldn't it be simpler just to check supported_archs for the presence of archs other than i386 or x86_64 and raise an error in that case?

comment:5 in reply to:  4 ; Changed 8 years ago by ryandesign (Ryan Carsten Schmidt)

Replying to rjvbertin@…:

qt5 portgroup which includes the muniversal portgroup which defines a universal variant.

This is always the case because a universal Qt5 build can only be generated that way.

I understand that. The problem is when the qt5 portgroup includes the muniversal portgroup and thus defines a universal variant for a port like qupzilla 2 which only builds for a single architecture -- x86_64 -- and thus should not have a universal variant.

It's customary to include portgroups at the top of the portfile, right after the portsystem line. supported_archs, if set at all, would typically be set later. But this is a problem for the qt5 portgroup which wants to include the muniversal portgroup if a universal build is going to happen. How can the qt5 portgroup know, at the time that it has been included, whether the portfile will later set supported_archs to a single value, or set universal_variant to no? And therefore how can the qt5 portgroup know whether or not it should include the muniversal portgroup? I don't know.

Regardless of that, wouldn't it be simpler just to check supported_archs for the presence of archs other than i386 or x86_64 and raise an error in that case?

I think that's an orthogonal issue, unrelated to this ticket.

comment:6 in reply to:  5 Changed 8 years ago by RJVB (René Bertin)

Replying to ryandesign@…:

The problem is when the qt5 portgroup includes the muniversal portgroup and thus defines a universal variant for a port like qupzilla 2 which only builds for a single architecture -- x86_64 -- and thus should not have a universal variant. And therefore how can the qt5 portgroup know whether or not it should include the muniversal portgroup? I don't know.

I guess there's only one way to come up with an answer to that: determine if Qt *applications* require the muniversal PG for them to be built. A quick test with qmake and cmake shows that once you manage to get -arch i386 -arch x8_64 into the C(XX)FLAGS straightforward universal binary builds work. That is, I get the expected universal binary .o files, and errors from the linker when it finds out that my Qt frameworks aren't universal.

If this is confirmed for more than the very simple applications I tested the muniversal inclusion can go into the Qt Portfile itself.

Even if it isn't confirmed, is there a way for the Qt PortGroup to oblige Portfiles to include muniversal if they want to support binary builds? I.e. can the Qt5 PortGroup remove the universal variant? It seems that the universal variant is available normally when one sets universal_variant no in the PortGroup, and then includes muniversal in a Portfile that wants to provide universal variants.

If you can confirm that is how things are supposed to work then it seems the best solution yet. It would avoid the question how well the cmake PortGroup handles +universal, and even less more you'd get qmake to do universal builds.

check supported_archs for the presence of archs other than i386 or x86_64 and raise an error in that case?

I think that's an orthogonal issue, unrelated to this ticket.

Probably, knowing now that your use case is x86_64 only.

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

I asked about "automatic universal builds" on the Qt ML, got this reply from Thiago Macieira:

We had deprecated that feature but brought it back with the iOS support and for the other Apple OSes (simulators, etc.).

So it probably works, if you lipo the Qt frameworks first.

So I'd propose the simple solution. Don't include muniversal and possibly set universal_variant no in the Qt5 PortGroup but in the Qt5 Portfile. Ports depending on Qt5 can include muniversal if they need +universal and the standard variant doesn't work for them.

comment:8 Changed 8 years ago by MarcusCalhoun-Lopez (Marcus Calhoun-Lopez)

Resolution: fixed
Status: assignedclosed

In bb693ed0/macports-ports:

qt5-1.0.tcl:

  • do not overwrite supported_archs
  • do not force universal variant

Fixes #51643
If Portfile defines merger-* procedure, muniversal PortGroup must be
explicitly used since muniversal would otherwise included later in the
Portfile evaluation.

Note: See TracTickets for help on using tickets.