Since bug 1002545 landed, we can't commit CSS files under Linux

RESOLVED FIXED

Status

Firefox OS
Gaia::Build
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: julienw, Assigned: julienw)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

46 bytes, text/x-github-pull-request
yurenju
: review+
Details | Review | Splinter Review
(Assignee)

Description

4 years ago
We get this error:

  .git/hooks/pre-commit: 100: .git/hooks/pre-commit: LD_LIBRARY_PATH=/home/julien/travail/git/gaia/b2g_sdk/34.0a1-2014-08-12-04-02-01/b2g/: not found

The issue is that the value we get under Linux as $XULRUNNER_SDK is now an environment variable assignment; however, the shell expands $XULRUNNER_SDK _after_ it adds leading specified environment variable, and as a result it takes the result of this expansion as a file name to execute.

The fix is really simple: simply prepend "env" to the command line.

Other uses are done inside Makefiles, so Makefile-based expansion kicks in before the command is launched in the shell, and that's why we didn't have this issue.
(Assignee)

Comment 1

4 years ago
Created attachment 8491579 [details] [review]
github PR
Attachment #8491579 - Flags: review?(yurenju)
(Assignee)

Comment 2

4 years ago
FTR I tested the fix on Linux but Rik was kind enough to test on MacOS that it was still working :)
(Assignee)

Comment 3

4 years ago
Comment on attachment 8491579 [details] [review]
github PR

Hey Alexandre, do you have some time to look at this?

Thanks!
Attachment #8491579 - Flags: review?(poirot.alex)
Comment on attachment 8491579 [details] [review]
github PR

I tried to solve it in other ways but does not work :-/

and |env| looks also a regular way to set a environment variables, so r=yurenju if we point out the reason which we use |env| in tools/pre-commit.
Attachment #8491579 - Flags: review?(yurenju)
Attachment #8491579 - Flags: review?(poirot.alex)
Attachment #8491579 - Flags: review+
(Assignee)

Comment 5

4 years ago
Added a comment and landed in:
master: 576ab24aeb3e53fdf6cc328ce7f038ad234b6d0b

Yeah, it's actually the first time I understand why "env" is useful without the "-i" parameter :)
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.