Opened 11 years ago

Closed 11 years ago

#23456 closed defect (fixed)

use_dmg yes fails when worksrcdir contains a space

Reported by: ryandesign (Ryan Schmidt) Owned by: ryandesign (Ryan Schmidt)
Priority: Normal Milestone: MacPorts 1.9.0
Component: base Version: 1.8.2
Keywords: Cc:
Port:

Description

This was noticed when trying to update BiggerSQL in #15634.

--->  Extracting BiggerSQL-1.3.9.dmg
DEBUG: setting option extract.args to /opt/local/var/macports/distfiles/BiggerSQL/BiggerSQL-1.3.9.dmg
DEBUG: Environment: CPATH='/opt/local/include' CC_PRINT_OPTIONS_FILE='/opt/local/var/macports/build/_Users_rschmidt_macports_dports_aqua_BiggerSQL/work/.CC_PRINT_OPTIONS' LIBRARY_PATH='/opt/local/lib' CC_PRINT_OPTIONS='YES' MACOSX_DEPLOYMENT_TARGET='10.6'
DEBUG: Assembled command: 'cd "/opt/local/var/macports/build/_Users_rschmidt_macports_dports_aqua_BiggerSQL/work" && /usr/bin/hdiutil attach /opt/local/var/macports/distfiles/BiggerSQL/BiggerSQL-1.3.9.dmg -private -readonly -nobrowse -mountpoint /tmp/mports.OXxglvqF/BiggerSQL/BiggerSQL Source && /bin/cp -Rp /tmp/mports.OXxglvqF/BiggerSQL/BiggerSQL Source /opt/local/var/macports/build/_Users_rschmidt_macports_dports_aqua_BiggerSQL/work && /usr/bin/hdiutil detach /tmp/mports.OXxglvqF/BiggerSQL/BiggerSQL Source && /bin/rmdir /tmp/mports.OXxglvqF/BiggerSQL/BiggerSQL Source /tmp/mports.OXxglvqF'
hdiutil: attach: extra image argument "Source" - "/opt/local/var/macports/distfiles/BiggerSQL/BiggerSQL-1.3.9.dmg" already specified
Usage:	hdiutil attach [options] <image>
	hdiutil attach -help
shell command " cd "/opt/local/var/macports/build/_Users_rschmidt_macports_dports_aqua_BiggerSQL/work" && /usr/bin/hdiutil attach /opt/local/var/macports/distfiles/BiggerSQL/BiggerSQL-1.3.9.dmg -private -readonly -nobrowse -mountpoint /tmp/mports.OXxglvqF/BiggerSQL/BiggerSQL Source && /bin/cp -Rp /tmp/mports.OXxglvqF/BiggerSQL/BiggerSQL Source /opt/local/var/macports/build/_Users_rschmidt_macports_dports_aqua_BiggerSQL/work && /usr/bin/hdiutil detach /tmp/mports.OXxglvqF/BiggerSQL/BiggerSQL Source && /bin/rmdir /tmp/mports.OXxglvqF/BiggerSQL/BiggerSQL Source /tmp/mports.OXxglvqF " returned error 1

The fix should be simple.

Attachments (2)

use_dmg.diff (2.5 KB) - added by ryandesign (Ryan Schmidt) 11 years ago.
use_dmg-raimue.diff (1.8 KB) - added by raimue (Rainer Müller) 11 years ago.

Download all attachments as: .zip

Change History (7)

Changed 11 years ago by ryandesign (Ryan Schmidt)

Attachment: use_dmg.diff added

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

Aaaaaaaaaaaa, I hate how non-simple this has turned out to be.

The simple part is the patch to portextract.tcl. I assumed ${worksrcdir} didn't contain spaces and used it in ${extract.post_args} directly. I should have put quotes around each use of this. It was also an error to use ${worksrcdir} at all; I should have used ${distname}. (In fact, I really wanted to use the actual name of the volume inside the disk image, but found that to read it I have to use "diskutil info -plist ${dmg_mount}", put that into somefile.plist, then read out the volume name with "defaults read somefile VolumeName". Besides being verbose and inconvenient, this only worked on Snow Leopard; on Leopard and Tiger, diskutil just showed its usage message. So I gave up on this for now.)

The problem is that even when I properly quote the paths, it doesn't help; ${extract.post_args} inexplicably loses the quotes I put there. Turns out this happens in portutil.tcl. I propose a patch to deal with this, but I'm unclear why the code was written the way it was written in the first place. I feel I'm basically undoing r628 with my change, and the commit message for r628 says it was done "to allow arguments ... to contain spaces" which seems to me to be the exact opposite of what it does. So I need some other eyes to look at this and see if there are any problems with my proposed changes before I commit them, since this is a change to a very fundamental part of MacPorts that's used by everything.

comment:2 in reply to:  1 Changed 11 years ago by ryandesign (Ryan Schmidt)

Replying to ryandesign@…:

So I need some other eyes to look at this and see if there are any problems with my proposed changes before I commit them

There are, exemplified by bzip2. Clearly my patch botches something. With patch, broken:

--->  Building bzip2
DEBUG: build phase started at Thu Feb 18 00:09:41 CST 2010
DEBUG: Executing proc-pre-org.macports.build-build-0
DEBUG: Executing org.macports.build (bzip2)
DEBUG: Environment: CPATH='/opt/local/include' CC_PRINT_OPTIONS_FILE='/opt/local/var/macports/build/_Users_rschmidt_macports_dports_archivers_bzip2/work/.CC_PRINT_OPTIONS' LIBRARY_PATH='/opt/local/lib' CC_PRINT_OPTIONS='YES' MACOSX_DEPLOYMENT_TARGET='10.6'
DEBUG: Assembled command: 'cd "/opt/local/var/macports/build/_Users_rschmidt_macports_dports_archivers_bzip2/work/bzip2-1.0.5" && /usr/bin/nice -n 10 /usr/bin/make all CC=\"/usr/bin/gcc-4.2 {-arch x86_64"} PREFIX=/opt/local'
sh: -c: line 0: unexpected EOF while looking for matching `"'
sh: -c: line 1: syntax error: unexpected end of file

Without patch, works fine:

--->  Building bzip2
DEBUG: build phase started at Thu Feb 18 00:10:57 CST 2010
DEBUG: Executing proc-pre-org.macports.build-build-0
DEBUG: Executing org.macports.build (bzip2)
DEBUG: Environment: CPATH='/opt/local/include' CC_PRINT_OPTIONS_FILE='/opt/local/var/macports/build/_Users_rschmidt_macports_dports_archivers_bzip2/work/.CC_PRINT_OPTIONS' LIBRARY_PATH='/opt/local/lib' CC_PRINT_OPTIONS='YES' MACOSX_DEPLOYMENT_TARGET='10.6'
DEBUG: Assembled command: 'cd "/opt/local/var/macports/build/_Users_rschmidt_macports_dports_archivers_bzip2/work/bzip2-1.0.5" && /usr/bin/nice -n 10 /usr/bin/make all CC="/usr/bin/gcc-4.2 -arch x86_64" PREFIX=/opt/local'

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

All options take lists as input. The problem is that Tcl splits the build.args in bzip2 in an unintended way.

See this example in tclsh:

% foreach i {FOO="foo bar" baz} { puts $i }
FOO="foo
bar"
baz
% foreach i {FOO="foo bar" "baz"} { puts $i }
FOO="foo
bar"
baz

Note that quotes around single list items are not preserved as they are only necessary to separate the list items. The code in command_string assembles a string from this format by converting each item to a string and then using string concatenation. List concatenation will not give the desired result.

It is also not possible to pass quoted arguments from a Portfile as handle_option will silently remove the quotes at the time of assignment. You would have to escape the quotes in this case.

Which leads to the solution to your initial problem. In order to pass the directories quoted, escape the quotes around such list items. I will attach a new patch which adds another level of backslashes.

Changed 11 years ago by raimue (Rainer Müller)

Attachment: use_dmg-raimue.diff added

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

Milestone: MacPorts 1.8.3MacPorts 1.9.0

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

Resolution: fixed
Status: newclosed

Committed in r67089.

Note: See TracTickets for help on using tickets.