Opened 12 years ago

Closed 11 years ago

Last modified 3 years ago

#17182 closed defect (fixed)

Variables aren't expanded in path:-style dependencies

Reported by: ryandesign (Ryan Schmidt) Owned by: macports-tickets@…
Priority: Normal Milestone: MacPorts 1.8.0
Component: base Version: 1.6.0
Keywords: Cc: markd@…, blb@…, simon@…, wsiegrist@…, jmroot (Joshua Root), MarcusCalhoun-Lopez (Marcus Calhoun-Lopez), pguyot (Paul Guyot)
Port:

Description

path:-style dependencies don't work as documented. The documentation shows this example:

depends_run         path:${prefix}/lib/libltdl.a:libtool

It turns out MacPorts doesn't actually expand variables here, so this doesn't work as intended.

Furthermore, it turns out MacPorts actually prepends "${prefix}/" if the given path does not begin with a slash.

So,

1) the Guide should show and recommend this simpler way to specify a dependency if it is in ${prefix}:

depends_run         path:lib/libltdl.a:libtool

And it could show a different example using variable expansion in the path, if the path is not in ${prefix} (a file in ${frameworks_dir} perhaps)

2) MacPorts base should be fixed to expand variables so this works as advertised

Ports that define dependencies using "path:${prefix}/..." should use "path:..." instead since this is shorter, simpler, and actually works with MacPorts 1.6.0.

Attachments (1)

macports.tcl.diff (882 bytes) - added by MarcusCalhoun-Lopez (Marcus Calhoun-Lopez) 11 years ago.

Download all attachments as: .zip

Change History (25)

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

Cc: markd@… blb@… added

comment:2 Changed 12 years ago by ryandesign (Ryan Schmidt)

Changed all my ports to the new style in r41807.

comment:3 Changed 12 years ago by ryandesign (Ryan Schmidt)

Changed all openmaintainer ports to the new style in r41808.

comment:4 in reply to:  3 Changed 12 years ago by ryandesign (Ryan Schmidt)

Replying to ryandesign@…:

Changed all openmaintainer ports to the new style in r41808.

Actually r41808 was for all nomaintainer ports. r41809 is for the openmaintainer ports.

comment:5 Changed 12 years ago by simon@…

Cc: simon@… wsiegrist@… added

I committed the doc fix in r41866 and r41871. But I didn't add an example for ${frameworks_dir} because I don't know how this works.

By the way, it looks like the guide isn't updated on commit anymore.

Simon

comment:6 Changed 12 years ago by wsiegrist@…

I broke guide regeneration at some point, but its fixed. Thanks for catching it for me.

comment:7 in reply to:  description Changed 12 years ago by blb@…

Replying to ryandesign@…:

And it could show a different example using variable expansion in the path, if the path is not in ${prefix} (a file in ${frameworks_dir} perhaps)

Note that frameworks_dir is in ${prefix} (at least by default, unless --with-frameworks-dir is used with configure). Once variable expansion works, then the example could be

path:${frameworks_dir}/MacPorts.framework/Versions/A/MacPorts:MacPorts_Framework

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

Cc: jmr@… added

Replying to jmr@...:

The problem with upgrading autoconf is actually that depspecs are auto-escaped, so variable substitution is not done, so it ends up looking for /opt/local/\${prefix}/bin/perl (because the prefix is prepended to any path that doesn't start with a slash.)

Joshua, could you tell me where in the source this is? I'm having a hard time finding it.

comment:9 in reply to:  8 Changed 12 years ago by blb@…

Replying to ryandesign@…:

Joshua, could you tell me where in the source this is? I'm having a hard time finding it.

I think he's referring to proc _pathtest.

comment:10 Changed 12 years ago by jmroot (Joshua Root)

This problem only occurs with upgrade, where the depspecs are taken from the PortIndex, and not with install, where they are taken directly from the Portfile. The backslash in \${prefix} is there in the PortIndex, so it looks like the bug is either in the portindex command, or in the way data is loaded from the PortIndex.

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

Cc: mcalhoun@… added

Cc Me!

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

Based on this discussion, could the problem be the following in portindex?:

--- portindex   2008-11-04 09:06:00.000000000 -0500                                                                                                                                                       
+++ portindex.new       2008-11-21 21:02:26.000000000 -0500                                                                                                                                               
@@ -38,7 +38,7 @@
     incr stats(total)                                                                                                                                                                                    
     global macports::prefix                                                                                                                                                                              
     set save_prefix $prefix                                                                                                                                                                              
-    set prefix {\${prefix}}                                                                                                                                                                              
+    set prefix {${prefix}}                                                                                                                                                                               
     if {[catch {set interp [mportopen file://[file join $directory $portdir]]} result]} {                                                                                                                
         puts stderr "Failed to parse file $portdir/Portfile: $result"                                                                                                                                    
         # revert the prefix.       

Making the change seems to have changed the \${prefix} to ${prefix} in a test Portindex file.

comment:13 in reply to:  12 ; Changed 12 years ago by ryandesign (Ryan Schmidt)

Replying to mcalhoun@…:

Making the change seems to have changed the \${prefix} to ${prefix} in a test Portindex file.

But then you're just setting ${prefix} to itself. Why is there code that goes to the trouble of saving the prefix, setting it to a backslashed version of the prefix, and restoring it later? That code was added in r14567. We would first need to fully understand why it was added then, to understand whether we can safely remove it now. Cc'ing Paul, who committed that revision.

comment:14 in reply to:  13 Changed 12 years ago by ryandesign (Ryan Schmidt)

Cc: pguyot@… added

Replying to ryandesign@…:

That code was added in r14567.

Actually it looks like it was added in r14532, bugfixed in r14545 and refined in r14567.

Paul, do you remember the motivation for and implication of this change?

comment:15 Changed 12 years ago by jmroot (Joshua Root)

I assume it's that way because the PortIndex is usually not generated on the local machine, so the prefix could be different.

comment:16 in reply to:  15 Changed 12 years ago by blb@…

Replying to jmr@…:

I assume it's that way because the PortIndex is usually not generated on the local machine, so the prefix could be different.

I believe that was exactly the reason.

comment:17 Changed 12 years ago by pguyot (Paul Guyot)

It looks like quite a dirty hack.... I believe it was indeed done to cope with PortIndex generation.

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

Please correct me if I am wrong, but it seems that the situation is:

  • portindex reads the Portfile for dependencies.
  • /opt/local should not be in the Portindex, so portindex MIGHT try
    set prefix {${prefix}}
    

before reading, which would result in ${prefix} in Portindex (instead of /opt/local).

  • Unfortunately, when Portindex is read, ${prefix} is evaluated to an empty string in mportsearch.
  • So what portindex actually does results in \${prefix} in Portindex.
  • Unfortunately, when the procedure _pathtest is finally called, the argument depspec still contains \${prefix}, which is never evaluated.
  • Therefore, _pathtest not only prepends the actual prefix, but, even if it didn't, would not work correctly because _mportsearchpath is searching for something with \${prefix} in it.

My Tcl is limited, so I do not know why

set line [read $fd $len]

behaves the way it does if $fd contains ${prefix} (not \${prefix}).

I tried a couple of things (like using eval) to have ${prefix} be evaluated in _pathtest, but all I could manage was to have an empty string for ${prefix}.

comment:19 Changed 12 years ago by jmroot (Joshua Root)

I think the command you want is subst. But I don't know why you'd be getting an empty string and not an error.

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

Attachment: macports.tcl.diff added

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

Replying to jmr@…:

I think the command you want is subst. But I don't know why you'd be getting an empty string and not an error.

I was not able to get subst to work.
Attached is a first attempt to correct the problem.
I know it's not pretty, but it was the only thing I could get to work.

The x11prefix (and eventually frameworks_dir and applications_dir) won't do anything without a modification to portindex.

comment:21 Changed 11 years ago by tobypeterson

Milestone: MacPorts base bugsMacPorts Future

Milestone MacPorts base bugs deleted

comment:22 Changed 11 years ago by jmroot (Joshua Root)

I think this may have been fixed "accidentally" by r44362, since it now grabs the portinfo from the portfile instead of using what's in the index. Will need to re-"break" a port and regenerate the index to test that theory, though.

comment:23 Changed 11 years ago by jmroot (Joshua Root)

Milestone: MacPorts FutureMacPorts 1.8.0
Resolution: fixed
Status: newclosed

Did the testing, looks fixed.

comment:24 Changed 3 years ago by neverpanic (Clemens Lang)

In 9a23db2/macports-base:

portindex: Restore special $prefix handling

This avoids expanded values for ${prefix} in the PortIndex.

Closes: #53169
See: #17182

Note: See TracTickets for help on using tickets.