Closed Bug 473413 Opened 11 years ago Closed 11 years ago

Build problem with spaces in path names

Categories

(NSPR :: NSPR, defect, P3)

x86
Windows XP
defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: wtc, Assigned: julien.pierre)

Details

Attachments

(2 files, 5 obsolete files)

Attached patch Proposed patch (obsolete) — Splinter Review
Jon Leighton reported this bug on the NSPR mailing list.

  I'm having a problem building NPSR in Windows XP when
  the path name contains spaces.  Changing the path to
  one without spaces solves the problem, but I need to
  be able to use paths with spaces.  The path below
  contains "Documents and Settings", which is causing
  the problem seen in the output below.  I'm using
  MozillaBuild 1.3.  Thank you for any help.

  - Jon


  $ make
  cd config; make -j1 export
  make[1]: Entering directory `/c/Documents and Settings/leighton/develo
  pment/firefox/source/tcp/3.0.5/config_test/nspr_test/config'
  cl -Fonow.obj -c      -W3 -nologo -GF -Gy -Od -MD -Z7   -UNDEBUG -DDEB
  UG_leighton -UWINNT  -DDEBUG=1 -DXP_PC=1 -DWIN32=1 -D_DEBUG=1 -DWIN95=
  1 -D_PR_GLOBAL_THREADS_ONLY=1 -D_X86_=1  -DFORCE_PR_LOG   /c/Documents
  and Settings/leighton/development/firefox/source/tcp/3.0.5/config_test
  /nsprpub/config/now.c
  cl : Command line warning D9024 : unrecognized source file type 'c:/Do
  cuments', object file assumed
  cl : Command line warning D9027 : source file 'c:/Documents' ignored
  cl : Command line warning D9024 : unrecognized source file type 'and', object file assumed
  cl : Command line warning D9027 : source file 'and' ignored
  now.c
  c1 : fatal error C1083: Cannot open source file: 'Settings/leighton/de
  velopment/firefox/source/tcp/3.0.5/config_test/nsprpub/config/now.c':
  No such file or directory
  make[1]: *** [now.obj] Error 2
  make[1]: Leaving directory `/c/Documents and Settings/leighton/develop
  ment/firefox/source/tcp/3.0.5/config_test/nspr_test/config'
  make: *** [export] Error

  $

I can reproduce this problem.  The attached patch fixes
the problem for me.  It quotes the C or C++ file path names
on Windows.  Jon, could you test it?
Attachment #356760 - Flags: review?(julien.pierre.boogz)
Attached patch Alternate Proposed Patch (obsolete) — Splinter Review
I attempted to apply Wan-Teh's patch manually and the build still failed.  I made an additional modification to rules.mk and the build completed without any issues.  This patch includes my attempt to manually apply Wan-Teh's patch as well as my additional modification.
Comment on attachment 356973 [details] [diff] [review]
Alternate Proposed Patch

Thanks, Jon.  I verified that your patch is the same
as mine.
Attachment #356973 - Attachment is obsolete: true
Yes, I see that now.  I missed the 2nd modification to Rules.mk when I tried to manually apply your patch - should've been more careful.  Thanks again for your quick response.
Comment on attachment 356760 [details] [diff] [review]
Proposed patch

r=nelson for Wan-Teh's patch.
Attachment #356760 - Flags: review+
I tried the patch and it solves the problem.

I think the quotes probably belong in the definition of abspath instead of the multiple callers. The patch only affects some of the callers. This was probably done to affect Windows only. I verified that added the quotes once in the abspath definition is OK on AIX too. I am not sure if they are OK on OS/2, which is the only other platform with NEED_ABSOLUTE_PATH defined, but I think so.

Unfortunately, when I tried to modify the abspath definition on Windows, I found out that that the MSYS version of gmake I'm using is horribly broken WRT function support. The "abspath" function always evaluates to its argument, no matter what I change the definition to. I can delete the definition altogether as well, and the behavior is the same. The version of MSYS I'm using 3.81.90  i686pc-pc-msys32 . Function support was supposed to have been added in gmake 3.80 and above, and it works on other platforms. I'm very concerned about this problem. Does anyone know of a version of make / gmake for Windows that properly supports functions ? The one I have is from the MozillaBuild package.
Attached patch Alternate patch (obsolete) — Splinter Review
Change abspath instead of its callers.
This patch worked after I upgraded to MozillaBuild 1.3 . The make bug with functions was fixed.
Attachment #360615 - Flags: review?
Comment on attachment 356760 [details] [diff] [review]
Proposed patch

This patch is OK, but I prefer the simpler and more generic alternate patch I attached.
Attachment #356760 - Flags: review?(julien.pierre.boogz) → review+
Actually, it turns out there are 2 versions of make in mozilla-build . One in c:\mozilla-build\msys\bin which is 3.79.1 and is OK with functions, and another in   c:\mozilla-build\msys\local\bin which is 3.81.90 and is broken with functions . When I upgraded my scripts switched from the later to the former and the MSYS make bug was hidden. Sigh.
See bug 476968 for the MozillaBuild make bug.
Attachment #360615 - Flags: review? → review+
Comment on attachment 360615 [details] [diff] [review]
Alternate patch

I agree that, if this patch works on all relevant platforms, it is preferable to changing all of the invokers.
Attached patch Better patch (obsolete) — Splinter Review
This patch also resolves the problem that I was running into, which is that abspath is a built-in function in some newer versions of gmake.
Attachment #356760 - Attachment is obsolete: true
Attachment #360615 - Attachment is obsolete: true
Attachment #361014 - Flags: review?
Attachment #361014 - Flags: review? → review?(wtc)
Attached patch Complete patch. (obsolete) — Splinter Review
Sorry, the previous patch was missing one file.
Attachment #361014 - Attachment is obsolete: true
Attachment #361016 - Flags: review?(wtc)
Attachment #361014 - Flags: review?(wtc)
Comment on attachment 361016 [details] [diff] [review]
Complete patch.

> ifdef NEED_ABSOLUTE_PATH
> PWD := $(shell pwd)
>-abspath = $(if $(findstring :,$(1)),$(1),$(if $(filter /%,$(1)),$(1),$(PWD)/$(1)))
>+core_abspath = "$(if $(findstring :,$(1)),$(1),$(if $(filter /%,$(1)),$(1),$(PWD)/$(1)))"
> endif

Please add a comment "The quotes allow spaces in the absolute pathnames."
and rename the function pr_abspath.  Then you can check this in.
The core_abspath name used in NSS was chosen because of the coreconf
build system.  For NSPR we should use the pr_ prefix.  Thanks.
Attachment #361016 - Flags: review?(wtc) → review-
Assignee: wtc → julien.pierre.boogz
Priority: -- → P3
Target Milestone: --- → 4.8
Checking in config/rules.mk;
/cvsroot/mozilla/nsprpub/config/rules.mk,v  <--  rules.mk
new revision: 3.67; previous revision: 3.66
done
Checking in pr/src/misc/Makefile.in;
/cvsroot/mozilla/nsprpub/pr/src/misc/Makefile.in,v  <--  Makefile.in
new revision: 1.20; previous revision: 1.19
Attachment #361016 - Attachment is obsolete: true
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
I backported the fix to the NSPR_4_7_BRANCH for NSPR 4.7.4.

Checking in config/rules.mk;
/cvsroot/mozilla/nsprpub/config/rules.mk,v  <--  rules.mk
new revision: 3.65.4.1; previous revision: 3.65
done
Checking in pr/src/misc/Makefile.in;
/cvsroot/mozilla/nsprpub/pr/src/misc/Makefile.in,v  <--  Makefile.in
new revision: 1.19.86.1; previous revision: 1.19
done
Target Milestone: 4.8 → 4.7.4
You need to log in before you can comment on or make changes to this bug.