Closed
Bug 392722
Opened 17 years ago
Closed 17 years ago
nsinstall chokes on double slashes in path
Categories
(Firefox Build System :: General, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: fta+bugzilla, Assigned: fta+bugzilla)
Details
Attachments
(1 file, 1 obsolete file)
2.33 KB,
patch
|
benjamin
:
review+
wtc
:
review+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a7) Gecko/2007081520 GranParadiso/3.0a7 Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a7) Gecko/2007081520 GranParadiso/3.0a7 nsinstall is not able to install in paths like /a/b//c/d if /a/b/ does not already exist. Problem is in mkdirs() which inserts a '\0' preventing 'c' and 'd' to be created. This has been found while building GranParadiso Alpha7 and trunk on Ubuntu. Here is what failed: nsinstall -D /src/firefox-granparadiso-3.0~alpha7/debian/tmp//usr/lib/firefox-granparadiso nothing after debian/tmp is created. Reproducible: Always Steps to Reproduce: 1. 2. 3. patch follows
Assignee | ||
Comment 1•17 years ago
|
||
patching both mozilla/nsprpub/config/nsinstall.c and mozilla/config/nsinstall.c
Comment 2•17 years ago
|
||
Confirmed ... the patch looks good.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Updated•17 years ago
|
Attachment #277231 -
Flags: review?(benjamin)
Assignee | ||
Comment 3•17 years ago
|
||
Add fix for a 3rd file (security/coreconf/nsinstall/nsinstall.c) causing FTBS in xulrunner on Ubuntu when using cdbs. Fix an issue with install -D foo (no slash at all) introduced by previous patch.
Attachment #277231 -
Attachment is obsolete: true
Attachment #277231 -
Flags: review?(benjamin)
Assignee | ||
Updated•17 years ago
|
Attachment #278285 -
Flags: review?(benjamin)
Updated•17 years ago
|
Summary: nsintall and double slashes → nsinstall chokes on double slashes in path
Comment 4•17 years ago
|
||
Comment on attachment 278285 [details] [diff] [review] fix multiple slashes bug, rev2 r=me for the config/nsinstall.c version, which I will land. The NSPR and NSS build systems are independent, and you'll need to get wtc/nelson to review and land those.
Attachment #278285 -
Flags: review?(benjamin) → review+
Comment 5•17 years ago
|
||
Comment on attachment 278285 [details] [diff] [review] fix multiple slashes bug, rev2 r=me for the config/nsinstall.c version, which I will land. The NSPR and NSS build systems are independent, and you'll need to get wtc/nelson to review and land those.
Comment 6•17 years ago
|
||
Checked in config/nsinstall.c Leaving this bug open for the NSPR and NSS versions.
Updated•17 years ago
|
Assignee: nobody → fta+bugzilla
Comment 7•17 years ago
|
||
Comment on attachment 278285 [details] [diff] [review] fix multiple slashes bug, rev2 requesting wtc's review for the nspr portion of the patch.
Attachment #278285 -
Flags: review?(wtc)
Comment 8•17 years ago
|
||
Checked in on trunk for coreconf (NSS) Checking in nsinstall.c; new revision: 1.9; previous revision: 1.8 Notice: this fix will affect all platforms except Windows and OS/2. (Does anyone know where the source for the Windows+OS/2 version is?)
Comment 9•17 years ago
|
||
Comment on attachment 278285 [details] [diff] [review] fix multiple slashes bug, rev2 r=wtc. Thanks for the patch. Please change the first line of the file to: /* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 4 -*- */ ^^^ ^^^ The for loop can be rewritten as: if ((cp = strrchr(path, '/'))) while (cp != path && cp[-1] == '/') cp--; Case 1: /a/b//c/d mkdirs("/a/b//c/d", mode) mkdirs("/a/b//c", mode) mkdirs("/a/b", mode) mkdirs("/a", mode) mkdir("/a", mode) mkdir("/a/b", mode) mkdir("/a/b//c", mode) mkdir("/a/b//c/d", mode) Case 2: /a/b/c/ (trailing /) mkdirs("/a/b/c/", mode) mkdirs("/a/b/c", mode) mkdirs("/a/b", mode) mkdirs("/a", mode) mkdir("/a", mode) mkdir("/a/b", mode) mkdir("/a/b/c", mode) mkdir("/a/b/c/", mode) <== EEXIST Fortunately we ignore EEXIST, originally for a different reason though.
Attachment #278285 -
Flags: review?(wtc) → review+
Comment 10•17 years ago
|
||
I checked in the patch on the NSPR trunk (NSPR 4.7). I made minor changes to the for loop: for (cp = strrchr(path, '/'); cp && cp != path && cp[-1] == '/'; cp--) ; Checking in nsinstall.c; /cvsroot/mozilla/nsprpub/config/nsinstall.c,v <-- nsinstall.c new revision: 3.22; previous revision: 3.21 done
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Updated•6 years ago
|
Component: Build Config → General
Product: Firefox → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•