Opened 8 years ago

Closed 6 years ago

Last modified 6 years ago

#32542 closed defect (fixed)

Setting configure.compiler to a MacPorts-provided compiler doesn't add dependency automatically

Reported by: neverpanic (Clemens Lang) Owned by: larryv (Lawrence Velázquez)
Priority: High Milestone: MacPorts 2.2.0
Component: base Version: 2.0.3
Keywords: Cc: quest@…, jeremyhu (Jeremy Huddleston Sequoia), skymoo (Adam Mercer), cooljeanius (Eric Gallager), nonstop.server@…
Port:

Description

Setting configure.compiler to a compiler provided my MacPorts doesn't automatically add a dependency on this compiler.

For an example see the percona port, which sets configure.compiler to apple-gcc-4.2, but doesn't declare a dependency on it.

Attachments (4)

auto-compiler-deps.diff (2.8 KB) - added by larryv (Lawrence Velázquez) 6 years ago.
Patch adding automatic dependency management for configure.compiler.
auto-compiler-deps.2.diff (3.9 KB) - added by larryv (Lawrence Velázquez) 6 years ago.
part deux
auto-compiler-deps.3.diff (4.2 KB) - added by larryv (Lawrence Velázquez) 6 years ago.
third time’s the charm
auto-compiler-deps.4.diff (3.2 KB) - added by neverpanic (Clemens Lang) 6 years ago.
4th attempt preventing the change of semantics of depends_lib and depends_build

Download all attachments as: .zip

Change History (42)

comment:1 Changed 8 years ago by ryandesign (Ryan Schmidt)

I think that's intentional.

comment:2 Changed 8 years ago by raimue (Rainer Müller)

I don't see the reason if this is intentional. If a compiler such as apple-gcc-4.2 is specified for configure.compiler, this explicitely requests the compiler installed by the MacPorts port. Thus, the apple-gcc-4.2 port should be in depends_build.

comment:3 Changed 8 years ago by raimue (Rainer Müller)

Actually, they need to be depends_lib in case produced binaries are linked to a runtime library like libgcc. So this is more complicated than I thought initially. See also #30041 for some reference.

comment:4 Changed 7 years ago by jeremyhu (Jeremy Huddleston Sequoia)

A build dependency should always be added for the port.

If it's gcc4X (not llvm-gcc42 and not apple-gcc4X), there should additionally be a lib dependency.

If the compiler is *clang* or apple-gcc42, it should skip_archcheck.

comment:5 Changed 7 years ago by jeremyhu (Jeremy Huddleston Sequoia)

Cc: jeremyhu@… added

Cc Me!

comment:6 Changed 7 years ago by jeremyhu (Jeremy Huddleston Sequoia)

This should really be done in 2.2

comment:7 Changed 7 years ago by raimue (Rainer Müller)

Owner: changed from raimue@… to macports-tickets@…

I thought I could implement this because I already did something similar for the use_autoconf stuff in the configure phase. I do not find any time to look into the issue at the moment, so anyone else might take this ticket.

comment:8 Changed 7 years ago by skymoo (Adam Mercer)

Cc: ram@… added

Cc Me!

comment:9 Changed 6 years ago by jeremyhu (Jeremy Huddleston Sequoia)

I'm just doing this in individual ports until this is solved:

# TODO: base should do this: http://trac.macports.org/ticket/32542
if {[portconfigure::compiler_is_port ${configure.compiler}]} {
    depends_build-append port:$portconfigure::compiler_name_map(${configure.compiler})
    depends_skip_archcheck-append $portconfigure::compiler_name_map(${configure.compiler})
}

comment:10 Changed 6 years ago by larryv (Lawrence Velázquez)

Cc: larryv@… added

Cc Me!

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

Cc: egall@… added

Cc Me!

comment:12 in reply to:  9 Changed 6 years ago by ryandesign (Ryan Schmidt)

Replying to jeremyhu@…:

I'm just doing this in individual ports until this is solved:

That seems fine, except for the gcc4x ports which need to be a lib dependency as noted above.

comment:13 Changed 6 years ago by jeremyhu (Jeremy Huddleston Sequoia)

Ah right. Crap. I'll need to update my block. Thanks for catching it.

But at least none of the default compilers provided in the default lists fall into that category.

comment:14 Changed 6 years ago by jeremyhu (Jeremy Huddleston Sequoia)

This should handle it then:

# TODO: base should do this: http://trac.macports.org/ticket/32542
if {[portconfigure::compiler_is_port ${configure.compiler}]} {
    depends_build-append port:$portconfigure::compiler_name_map(${configure.compiler})

    # base 2.1.x ignores the argument and just use ${configure.compiler}
    if {[portconfigure::arch_flag_supported ${configure.compiler}]} {
        depends_skip_archcheck-append $portconfigure::compiler_name_map(${configure.compiler})
    }

    if {[string match macports-gcc* ${configure.compiler}]} {
        depends_lib-append port:$portconfigure::compiler_name_map(${configure.compiler})
    }
}

comment:15 Changed 6 years ago by jeremyhu (Jeremy Huddleston Sequoia)

The workaround above causes issues with the generated PortIndex on the rsync servers, #37817

What is a viable workaround? As noted in the release notes for XCode 4.6, this is the last release to contain llvm-gcc, so we're quickly approaching the point where *any* backup compiler will be a port.

comment:16 Changed 6 years ago by jeremyhu (Jeremy Huddleston Sequoia)

Priority: NormalHigh

comment:17 Changed 6 years ago by larryv (Lawrence Velázquez)

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

I’m working on it.

comment:18 Changed 6 years ago by jeremyhu (Jeremy Huddleston Sequoia)

Awesome, thanks.

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

Cc: larryv@… removed

Cc Me!

Changed 6 years ago by larryv (Lawrence Velázquez)

Attachment: auto-compiler-deps.diff added

Patch adding automatic dependency management for configure.compiler.

comment:20 Changed 6 years ago by larryv (Lawrence Velázquez)

I’ve implemented this as an option_proc on configure.compiler. It should work for both the default value (generated from blacklists, whitelists, and the fallback list) and for explicit overrides. Just in case, it handles some bizarre cases like multiple overrides and unsetting the variable completely.

It’s worked well in my limited testing; let me know about any issues I overlooked.

comment:21 Changed 6 years ago by nonstop.server@…

Cc: nonstop.server@… added

Cc Me!

comment:22 Changed 6 years ago by jeremyhu (Jeremy Huddleston Sequoia)

Thanks, I'll give it a whirl this week.

The logic looks sound, but my tcl-foo is not quite there to know if this is the best approach, so I'll let someone else comment on that aspect.

comment:23 Changed 6 years ago by jeremyhu (Jeremy Huddleston Sequoia)

That change works if I set configure.compiler, but not if I use the blacklist:

# This doesn't pull in the dependency on llvm-gcc42
compiler.blacklist  clang

# This pulls in the dependency
configure.compiler  macports-llvm-gcc-4.2

comment:24 in reply to:  23 Changed 6 years ago by larryv (Lawrence Velázquez)

Replying to jeremyhu@…:

That change works if I set configure.compiler, but not if I use the blacklist:

Huh. I tested that on ld64 by changing the existing blacklist from “gcc-4.0” to “clang” and commenting out your workaround, but if I add a blacklist to zsh, nothing shows up. Will take a look shortly.

Last edited 6 years ago by larryv (Lawrence Velázquez) (previous) (diff)

comment:25 Changed 6 years ago by larryv (Lawrence Velázquez)

Ugh, looks like I can't even rely on configure.compiler being read once before dependencies are accessed. I’ll have to find a better place to put this.

Changed 6 years ago by larryv (Lawrence Velázquez)

Attachment: auto-compiler-deps.2.diff added

part deux

comment:26 Changed 6 years ago by larryv (Lawrence Velázquez)

Alright, let’s try this again. I make the same claims as last time, except this time they should be true!

Changed 6 years ago by larryv (Lawrence Velázquez)

Attachment: auto-compiler-deps.3.diff added

third time’s the charm

comment:27 Changed 6 years ago by larryv (Lawrence Velázquez)

Patch The Third has basically the same logic as the 2nd, but I rearranged things to clarify the control flow.

comment:28 Changed 6 years ago by jeremyhu (Jeremy Huddleston Sequoia)

Awesome! I'll give it a whirl in a bit.

comment:29 Changed 6 years ago by jeremyhu (Jeremy Huddleston Sequoia)

Awesome. I tried a small test case with each of:

configure.compiler  macports-llvm-gcc-4.2
compiler.whitelist  macports-llvm-gcc-4.2
compiler.blacklist  clang

and in each case, it pulled in llvm-gcc42 as a dependency! Good job. Please give some time for jmr or others to comment on the actual implementation (esp hijack_option), and let's get this in sometime this week =)

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

hijack_option does change the semantics of depends_lib and depends_build. Although it probably doesn't make a difference in most cases we should consider doing this without this hack. If we had a callback that would be evaluated after the Portfile (and all variants) were completely sourced, we could add the correct compiler dependency there using a simple depends_build-append statement.

source:trunk/base/src/macports1.0/macports.tcl@102897:1608-1618#L1608 is the relevant part of the code executing the Portfiles. Adding a callback there (e.g., $workername eval add_compiler_dependencies, which would in turn have to be created using $workername alias add_compiler_dependencies actually_implementing_procedure) seems like a cleaner solution to me and would avoid the change in semantics. We could also provide a generic callback mechanism there, just in case we (or a Portfile author) ever needs something similar.

comment:31 Changed 6 years ago by neverpanic (Clemens Lang)

Working on a patch.

comment:32 Changed 6 years ago by larryv (Lawrence Velázquez)

I got it, Cal. I was actually initially looking for a place to run this after the Portfile was done being sourced, but I gave up on mucking through macports1.0/ and settled on the variable trace. This is exactly what I wanted, though.

Last edited 6 years ago by larryv (Lawrence Velázquez) (previous) (diff)

Changed 6 years ago by neverpanic (Clemens Lang)

Attachment: auto-compiler-deps.4.diff added

4th attempt preventing the change of semantics of depends_lib and depends_build

comment:33 Changed 6 years ago by neverpanic (Clemens Lang)

Attached a patch providing a generic callback mechanism (that might also be useful in other cases, I recall we also have an automatic dependcy on bsdmake if ${build.type} == bsd where this could be useful) and automatic compiler dependencies.

To prevent duplicate dependencies in ports that already correctly declare a dependency we might add depends_${type}-delete port:$compiler_port, but that's just a cosmetic change now.

Comments?

comment:34 Changed 6 years ago by larryv (Lawrence Velázquez)

Looks good to me. Wish I’d known where mportopen was before I spent so much time making those traces work properly. C’est la vie.

The use_auto* options would probably benefit from using callbacks, too.

comment:35 Changed 6 years ago by jeremyhu (Jeremy Huddleston Sequoia)

That looks great, thanks.

comment:36 Changed 6 years ago by neverpanic (Clemens Lang)

#37817 reminds me this will not be working properly on the rsync server when generating the index in some cases: The portindex is generated on a linux box. portindex supports pretending to be on another platform (which is how we achieve platform-specific indices). However, on those machines, $xcodeversion is always "none", causing compiler.fallback to be {cc} by default.

So, e.g., for ports on 10.8 blacklisting all xcode compilers the fallback would be some macports compiler, causing a dependency to be added. On the server generating the portindex the fallback (and default anyway) would be cc. Thus, the dependency wouldn't get recorded in the PortIndex.

If we wanted to fix this correctly, we'd have to pretend having Xcode installed on the servers, too. This would also mean that our PortIndex would be Xcode-version-specific and we'd have to support multiple versions of Xcode per OS version. I'd say it's not worth the effort and we can live with somewhat off dependencies in the PortIndex (since building will still work correctly).

comment:37 Changed 6 years ago by neverpanic (Clemens Lang)

Milestone: MacPorts Future
Resolution: fixed
Status: assignedclosed

Commited in r102932. Thanks to larryv and jeremyhu for the patches and testing.

comment:42 Changed 6 years ago by jmroot (Joshua Root)

Milestone: MacPorts FutureMacPorts 2.2.0
Note: See TracTickets for help on using tickets.