This can cause us to break with unhelpful error messages like: /bin/sh: ../../dist/xpi-stage/gban: File exists make: *** [libs] Error 1 make: *** [all] Error 2
I didn't test this, but it's pretty simple. We might even consider erroring if XPI_NAME is set but contains spaces.
Attachment #255988 - Flags: review?(benjamin)
(In reply to comment #1) > Created an attachment (id=255988) [details] > strip XPI_NAME > > I didn't test this, but it's pretty simple. We might even consider erroring if > XPI_NAME is set but contains spaces. > I've tested this and it works.
Comment on attachment 255988 [details] [diff] [review] strip XPI_NAME After discussion on IRC, we're going to try and create a meaningful error if various important variables have extraneous spaces. This might be hard ;-)
Attachment #255988 - Flags: review?(benjamin) → review-
Summary: strip trailing spaces from XPI_NAME when setting FINAL_TARGET → warn on trailing spaces in XPI_NAME (breaks FINAL_TARGET)
This checks XPI_NAME for internal or trailing spaces. Are there other vars you think we should sanity check while I'm doing this?
Comment on attachment 256032 [details] [diff] [review] warn if spaces or trailing spaces in XPI_NAME LIBRARY_NAME MODULE DEPTH SHORT_LIBNAME XPI_PKGNAME INSTALL_EXTENSION_ID You can probably shorten this to: $(if $(filter-out 0 1,$(words $(XPI_NAME))),$(error Spaces are not allowed in XPI_NAME)) And then you could do a $(foreach) to do that for a bunch of variables. Though... that's not pretty.
Attachment #256032 - Flags: review?(benjamin) → review-
Okay, this checks all the vars that bsmedberg mentioned, and uses a slightly modified version of his filter-out example to do so.
Comment on attachment 259684 [details] [diff] [review] check a bunch of variables >Index: config/config.mk >+CHECK_VARS := XPI_NAME \ >+ LIBRARY_NAME \ I think we should standardize on a whitespace strategy, and I'd prefer SETVAR = \ THING1 \ THING2 \ $(NULL) We would only use spaces where absolutely required by rule syntax, and if we change the name of a variable we don't end up changing the spacing. Other than that, r=me. Thanks.
Attachment #259684 - Flags: review?(benjamin) → review+
Checked in with that fix.
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
Summary: warn on trailing spaces in XPI_NAME (breaks FINAL_TARGET) → error on spaces in XPI_NAME and some other Makefile vars
You need to log in before you can comment on or make changes to this bug.