125962 - broken relocates in pkgcp.pl on Win for wildcards and dirs

Status: RESOLVED FIXED

(SeaMonkey :: Build Config, defect)

Description

[Daniel Veditz [:dveditz]]

The package list relocation syntax ("src,dest") is broken on Win32, it only
works when the src is a single file. Using wildcard syntax it silently does nothing

  ex: bin\searchplugins\*,searchplugins

and when source is a directory the script dies

  ex: bin\searchplugins,searchplugins

Comment 1

[Daniel Veditz [:dveditz]]

nominating because this blocks an nsbeta1+ bug.

My first thought was that if the wildcard syntax was broken maybe the directory
syntax would work. I quickly found a "#Todo: add MSDOS hack" that was never done
and moved the ugly hack that worked with the regular directory
copy (no alternate destination) and tweaked it as required. I'm just guessing at
the OS/2 version -- the existing OS/2 hack didn't make sense to me so my
alterations might be completely wrong.

The wildcards turned out to be a simple missing bracket, turns out I should have
just started with that one :-)

With my patch both forms will now work on Windows as they do on Unix. Someone
should check out OS/2. In some ways OS/2 being broken would be no worse than
before, but with Win32 working we might be tempted to start using one or the
other of these syntaxes (to fix bug 93140, for instance).

Comment 2

[Daniel Veditz [:dveditz]]

Attachment: fix for pkgcp.pl

Comment 3

[dbragg]

Comment on attachment 69909 [details] [diff] [review]
fix for pkgcp.pl

I love slash substituions in Perl. r=dbragg

Comment 4

[jhp (no longer active)]

What is a good way to test this?  I am working on the OS/2 version, and I've
been testing it like this: "perl pkgcp.pl --debug 10 -s ..\..\obj\dist -d javier
-f packages-win -o OS2".  However, this causes errors in both OS/2 and Win32
(using "dos" instead of "OS2").  Any tips? 

Comment 5

[Daniel Veditz [:dveditz]]

Try "perl pkgcp.pl --help" for the gory details, but you MUST use absolute paths
for source and destination. Then I'd add a test component to packages-win so you
can test the fix as well as verify the old stuff still works:

  [testme]
  bin\searchplugins\*
  bin\searchplugins,searchdir
  bin\searchplugins\*,searchwild

You can use the "-c' option to test only a single component

Comment 6

[jhp (no longer active)]

Attachment: os/2 changes

OK, here are the changes I needed to get things to work on OS/2.  I'm not sure
if everything is in the right place;  help is appreciated.  This patch includes
the changes from the previous patch.

Comment 7

[Jon Granrose]

Every time I look at pkgcp.pl I'm amazed it works at all... I was definitely in
an altered state when I hacked that out.

I'm probably not the right person to sr this since the MSDOS hack looks ok, but
I have no idea what adding the {} around the $PD does, and my Programming Perl
doesn't offer a clue.  Can someone clue me in?

Comment 8

[Daniel Veditz [:dveditz]]

Now it's my turn to be amazed that this actually works: I meant to use () parens
to group things; {} are used for quantifiers. Ex: a{3} would match aaa.

The problem I was trying to solve was that the old expression s/$PD*$// was
matching the literal * and removing the wildcard-ness. In fact that turned it
into a legitimate directory expression, but that was broken too, thus the other
hack. But still, why was it removing the *?

My patch 'worked' because it doesn't match anything. It isn't taking the * out
of the test case we worried about, but it isn't stripping end slashes as it was
intended to do (neither did the original code, though).

I have no idea why \\* was behaving like \*, although in another place s/^$PD//
on unix works rather than giving the syntax error I'd expect from s/^/// --
obviously that's getting interpreted as s/^\/// so why isn't the second
backslash added on dos?

I just tried using parens and I get a syntax error about an unmatched paren,
indicating that it thought my expression was s/(\)*// -- "\)" being a literal
paren and not matching the grouping paren.

Arg!

Comment 9

[Jon Granrose]

Interesting.  One thing to be aware of in pkgcp.pl is that some of the recursive
items like copying or removing directories use File::Find which produces paths
with forward slashes (/) as separators on DOS which is why there's a routine in
there to flip slashes, so you have to always check what the PD is at any given
time when comparing search/replace calls.

I don't know if this would help, but looking at pg 73 of Programming Perl it
says you can use naturally paired delimiters like () and {} for search/replace
e.g.  s(${PD}*)()

Comment 10

[selmer (gone)]

Please update this bug with an [adt1] - [adt3] impact rating (or take it off the
list if it doesn't even rate adt3.)  Thanks!

Comment 11

[jhp (no longer active)]

Attachment: update patch to trunk

The code that is touched by the patch has been moved from pkgcp.pl to
Packager.pm.  Redoing patch.

Comment 12

[Daniel Veditz [:dveditz]]

Comment on attachment 77704 [details] [diff] [review]
update patch to trunk

This patch must need an update given the patch in bug 135508 that removes os2
as an $os type (plus the $line variable).

Comment 13

[Mike Kaply [:mkaply]]

This fix went in for windows and we were keeping it open for an OS/2 change.

We decided to just take the Unix path on OS/2 in Packager.pm, so this change
isn't needed anymore.