Skip Menu |
Report information
Id: 126834
Status: resolved
Priority: 0/
Queue: perl5

Owner: Nobody
Requestors: Stromeko [at] nexgo.de
Cc:
AdminCc:

Operating System: (no value)
PatchStatus: (no value)
Severity: low
Type: unknown
Perl Version: (no value)
Fixed In: (no value)



Date: Sun, 06 Dec 2015 18:57:33 +0100
Subject: Cygwin cygdrive prefix test
To: perlbug [...] perl.org
From: Achim Gratz <Stromeko [...] nexgo.de>
Download (untitled) / with headers
text/plain 336b
The test for the cygdrive prefix can be simplified by using the cygpath executable instead of parsing the output from df or mount. An alternative for modern Cygwin would be to use my $prefix2 = readlink "/proc/cygdrive" if a fork and some parsing should be avoided and supporting older Cygwin versions is not considered important.
--- t/lib/cygwin.t.orig 2015-12-06 18:29:01.970308000 +0100 +++ t/lib/cygwin.t 2015-12-06 18:44:29.329880800 +0100 @@ -51,18 +51,14 @@ # Cygdrive mount prefix my @flags = split(/,/, Cygwin::mount_flags('/cygdrive')); -my $prefix = pop(@flags); -ok($prefix, "cygdrive mount prefix = " . (($prefix) ? $prefix : '<none>')); -chomp(my $prefix2 = `df -a | grep -i '^c: ' | cut -d% -f2 | xargs`); -# we get something like "C: - - - - /cygdrive" if this isn't the entry -# df displays free space info for -$prefix2 =~ s/.* //; -$prefix2 =~ s/\/c$//i; +my $prefix1 = pop(@flags); +ok($prefix1, "cygdrive mount prefix1 = " . (($prefix1) ? $prefix1 : '<none>')); +chomp(my $prefix2 = `cygpath C:`); # the drive need not actually exist, so this will always work +$prefix2 =~ s/\/c$//; +$prefix2 = "/" unless $prefix2; SKIP: { - $prefix2 - or skip("No C: entry found in df output", 1); - is($prefix, $prefix2, 'cygdrive mount prefix'); + is($prefix1, $prefix2, 'cygdrive mount prefix2 = ' . $prefix2); } my @mnttbl = Cygwin::mount_table();
Download (untitled) / with headers
text/plain 187b
Regards, Achim. -- +<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+ Wavetables for the Terratec KOMPLEXER: http://Synth.Stromeko.net/Downloads.html#KomplexerWaves
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 548b
On Sun Dec 06 09:57:49 2015, Stromeko@nexgo.de wrote: Show quoted text
> > The test for the cygdrive prefix can be simplified by using the cygpath > executable instead of parsing the output from df or mount. An > alternative for modern Cygwin would be to use > > my $prefix2 = readlink "/proc/cygdrive" > > if a fork and some parsing should be avoided and supporting older Cygwin > versions is not considered important. >
The change looks sane, though I'm not sure why you renamed $prefix. Can you supply this patch as a "git format-patch" style patch? Tony
To: perl5-porters [...] perl.org
Subject: Re: [perl #126834] Cygwin cygdrive prefix test
From: Achim Gratz <Stromeko [...] nexgo.de>
Date: Mon, 07 Dec 2015 07:59:25 +0100
Download (untitled) / with headers
text/plain 443b
Tony Cook via RT writes: Show quoted text
> The change looks sane, though I'm not sure why you renamed $prefix.
Just for symmetry reasons, but I have no problem with keeping it named just $prefix. Show quoted text
> Can you supply this patch as a "git format-patch" style patch?
Can do. Regards, Achim. -- +<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+ Samples for the Waldorf Blofeld: http://Synth.Stromeko.net/Downloads.html#BlofeldSamplesExtra
To: perl5-porters [...] perl.org
From: Achim Gratz <Stromeko [...] nexgo.de>
Date: Mon, 07 Dec 2015 19:48:37 +0100
Subject: Re: [perl #126834] Cygwin cygdrive prefix test
Download (untitled) / with headers
text/plain 102b
Tony Cook via RT writes: Show quoted text
> Can you supply this patch as a "git format-patch" style patch?
Attached.
From b12ceb2c68415fbe6e46390f71870fb73854f24a Mon Sep 17 00:00:00 2001 From: Achim Gratz <Stromeko@NexGo.DE> Date: Mon, 7 Dec 2015 19:42:37 +0100 Subject: [PATCH] [perl #126834] Cygwin cygdrive prefix test * t/lib/cygwin.t: The test for the cygdrive prefix is simplified by using the cygpath executable instead of parsing the output from df or mount. An alternative for modern Cygwin would be to use my $prefix2 = readlink "/proc/cygdrive" if a fork and some parsing should be avoided and supporting older Cygwin versions is not considered important. --- t/lib/cygwin.t | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/t/lib/cygwin.t b/t/lib/cygwin.t index 9033d3f..174f3bd 100644 --- a/t/lib/cygwin.t +++ b/t/lib/cygwin.t @@ -52,13 +52,14 @@ is(Cygwin::mount_flags("/cygdrive") =~ /,cygdrive/, 1, "check cygdrive mount_fl # Cygdrive mount prefix my @flags = split(/,/, Cygwin::mount_flags('/cygdrive')); my $prefix = pop(@flags); -ok($prefix, "cygdrive mount prefix = " . (($prefix) ? $prefix : '<none>')); -chomp(my $prefix2 = `df | grep -i '^c: ' | cut -d% -f2 | xargs`); -$prefix2 =~ s/\/c$//i; -if (! $prefix2) { - $prefix2 = '/'; +ok($prefix, "cygdrive mount prefix = " . (($prefix) ? $prefix : '<none>')); +chomp(my $prefix2 = `cygpath C:`); # the drive need not actually exist, so this will always work +$prefix2 =~ s/\/c$//; +$prefix2 = "/" unless $prefix2; +SKIP: +{ + is($prefix, $prefix2, 'cygdrive mount prefix2 = ' . $prefix2); } -is($prefix, $prefix2, 'cygdrive mount prefix'); my @mnttbl = Cygwin::mount_table(); ok(@mnttbl > 0, "non empty mount_table"); -- 2.6.2
Download (untitled) / with headers
text/plain 192b
Regards, Achim. -- +<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+ SD adaptation for Waldorf rackAttack V1.04R1: http://Synth.Stromeko.net/Downloads.html#WaldorfSDada
CC: perl5-porters [...] perl.org
To: Achim Gratz <Stromeko [...] nexgo.de>
Subject: Re: [perl #126834] Cygwin cygdrive prefix test
From: Ivan Pozdeev via perl5-porters <perl5-porters [...] perl.org>
Date: Mon, 7 Dec 2015 22:58:53 +0300
Download (untitled) / with headers
text/plain 961b
`cygpath C:\' isn't completely fool-proof as C:\ might be mounted independently. (you decide if it's fool-proof enough for the case at hand) `readlink /proc/cygdrive' does look like the best solution. So, the best way I see is to use it and fall back to the heuristics if it's missing. "Shelling out" can't be avoided unless you wish to check the registry by hand: initially (in 2000), there was no dedicated utility to read the prefix at all, the docs pointed directly to the registry (http://web.archive.org/web/20001213205800/http://www.cygwin.com/cygwin-ug-net/using.html; http://web.archive.org/web/20001213195700/http://www.cygwin.com/cygwin-ug-net/using-utils.html#MOUNT). I'll now check the Cygwin repo for /proc/cygdrive addition time vs. registry setting elimination time to see if one can avoid the heuristics entirely and skip from the former directly to the latter. -- Best regards, Ivan mailto:vano@mail.mipt.ru
Date: Mon, 07 Dec 2015 21:36:10 +0100
Subject: Re: [perl #126834] Cygwin cygdrive prefix test
From: Achim Gratz <Stromeko [...] nexgo.de>
To: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 676b
Ivan Pozdeev via perl5-porters writes: Show quoted text
> `cygpath C:\' isn't completely fool-proof as C:\ might be mounted independently. > (you decide if it's fool-proof enough for the case at hand)
In which case the former code would fail just the same. If you wanted, you could loop through all 26 drive letters and see which prefix comes up the most often and pick that as the cygdrive prefix, but it could still fail in some contrived situations. Show quoted text
> `readlink /proc/cygdrive' does look like the best solution. > > So, the best way I see is to use it and fall back to the heuristics if it's > missing.
Well, I can get behind that idea; thanks for mentioning. How about the attached?

Message body is not shown because sender requested not to inline it.

Download (untitled) / with headers
text/plain 208b
Regards, Achim. -- +<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+ Factory and User Sound Singles for Waldorf Q+, Q and microQ: http://Synth.Stromeko.net/Downloads.html#WaldorfSounds
Date: Mon, 07 Dec 2015 21:43:43 +0100
Subject: Re: [perl #126834] Cygwin cygdrive prefix test
From: Achim Gratz <Stromeko [...] nexgo.de>
To: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 202b
Achim Gratz writes: Show quoted text
> Well, I can get behind that idea; thanks for mentioning. How about the > attached?
Sorry, the patch still had a change in I made to test the fallback… Here's the correct one.

Message body is not shown because sender requested not to inline it.

Download (untitled) / with headers
text/plain 208b
Regards, Achim. -- +<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+ Factory and User Sound Singles for Waldorf Q+, Q and microQ: http://Synth.Stromeko.net/Downloads.html#WaldorfSounds
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 326b
On Mon Dec 07 12:44:47 2015, Stromeko@nexgo.de wrote: Show quoted text
> Achim Gratz writes:
> > Well, I can get behind that idea; thanks for mentioning. How about the > > attached?
> > Sorry, the patch still had a change in I made to test the fallback… > Here's the correct one.
This patch doesn't seem to be against blead perl. Tony
Date: Wed, 09 Dec 2015 19:01:56 +0100
Subject: Re: [perl #126834] Cygwin cygdrive prefix test
From: Achim Gratz <Stromeko [...] nexgo.de>
To: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 123b
Tony Cook via RT writes: Show quoted text
> This patch doesn't seem to be against blead perl.
Sorry, you're right -- should be fixed now.
From 91acd29f061acccda0c13e8f47e0d82f45eea6c5 Mon Sep 17 00:00:00 2001 From: Achim Gratz <Achim.Gratz@Stromeko.DE> Date: Wed, 9 Dec 2015 18:59:03 +0100 Subject: [PATCH] [perl #126834] Cygwin cygdrive prefix test * t/lib/cygwin.t: Use the /proc virtual filesystem to determine the cygdrive prefix. If that isn't available, fall back to using the cygpath executable instead of parsing the output from df or mount for older Cygwin. That fallback can fail if C:\ is manually mounted someplace else, but the former code had the same problem. --- t/lib/cygwin.t | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/t/lib/cygwin.t b/t/lib/cygwin.t index a619e40..ba86170 100644 --- a/t/lib/cygwin.t +++ b/t/lib/cygwin.t @@ -52,18 +52,16 @@ is(Cygwin::mount_flags("/cygdrive") =~ /,cygdrive/, 1, "check cygdrive mount_fl # Cygdrive mount prefix my @flags = split(/,/, Cygwin::mount_flags('/cygdrive')); my $prefix = pop(@flags); -ok($prefix, "cygdrive mount prefix = " . (($prefix) ? $prefix : '<none>')); -chomp(my $prefix2 = `df -a | grep -i '^c: ' | cut -d% -f2 | xargs`); -# we get something like "C: - - - - /cygdrive" if this isn't the entry -# df displays free space info for -$prefix2 =~ s/.* //; -$prefix2 =~ s/\/c$//i; -SKIP: -{ - $prefix2 - or skip("No C: entry found in df output", 1); - is($prefix, $prefix2, 'cygdrive mount prefix'); +ok($prefix, "cygdrive mount prefix = " . (($prefix) ? $prefix : '<none>')); +my $prefix2 = readlink "/proc/cygdrive"; +unless ($prefix2) { + # fallback to old Cygwin, the drive need not actually exist, so + # this will always work (but might return the wrong prefix if the + # user re-mounted C:\ + chomp($prefix2 = `cygpath C:`); + $prefix2 = substr($prefix2, 0, -1-(length($prefix2)>2)); } +is($prefix, $prefix2, 'cygdrive mount prefix2 = ' . $prefix2); my @mnttbl = Cygwin::mount_table(); ok(@mnttbl > 0, "non empty mount_table"); -- 2.6.2
Download (untitled) / with headers
text/plain 191b
Regards, Achim. -- +<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+ Wavetables for the Waldorf Blofeld: http://Synth.Stromeko.net/Downloads.html#BlofeldUserWavetables
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 324b
On Wed Dec 09 10:02:45 2015, Stromeko@nexgo.de wrote: Show quoted text
> Tony Cook via RT writes:
> > This patch doesn't seem to be against blead perl.
> > Sorry, you're right -- should be fixed now. >
Thanks, applied as 03e4d23263b0403a1e4007b74fc4e9c511cd54a8. Added you as an author in 772a2be508732f9a1684159914cdd6a9105526cc. Tony
From: Achim Gratz <Stromeko [...] nexgo.de>
Date: Thu, 10 Dec 2015 10:34:26 +0100
To: perl5-porters [...] perl.org
Subject: Re: [perl #126834] Cygwin cygdrive prefix test
Download (untitled) / with headers
text/plain 303b
Tony Cook via RT writes: Show quoted text
> Thanks, applied as 03e4d23263b0403a1e4007b74fc4e9c511cd54a8.
Thank you. Regards, Achim. -- +<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+ Factory and User Sound Singles for Waldorf rackAttack: http://Synth.Stromeko.net/Downloads.html#WaldorfSounds
Download (untitled) / with headers
text/plain 252b
Thank you for submitting this report. You have helped make Perl better. With the release of Perl 5.24.0 on May 9, 2016, this and 149 other issues have been resolved. Perl 5.24.0 may be downloaded via https://metacpan.org/release/RJBS/perl-5.24.0


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

For issues related to this RT instance (aka "perlbug"), please contact perlbug-admin at perl.org