Bug 82209

Summary: [jhbuild] Use $MAKE if it is defined to build jhbuild itself.
Product: WebKit Reporter: Raphael Kubo da Costa (:rakuco) <rakuco>
Component: Tools / TestsAssignee: Raphael Kubo da Costa (:rakuco) <rakuco>
Status: RESOLVED FIXED    
Severity: Normal CC: d-r, gustavo, mrobinson, pnormand
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch mrobinson: review+

Description Raphael Kubo da Costa (:rakuco) 2012-03-26 07:44:59 PDT
[jhbuild] Use $MAKE if it is defined to build jhbuild itself.
Comment 1 Raphael Kubo da Costa (:rakuco) 2012-03-26 07:46:01 PDT
Created attachment 133812 [details]
Patch
Comment 2 Gustavo Noronha (kov) 2012-03-26 08:23:00 PDT
Comment on attachment 133812 [details]
Patch

A few lines before that we strip MAKE out, so this won't do good.
Comment 3 Raphael Kubo da Costa (:rakuco) 2012-03-26 08:30:10 PDT
Actually, I wonder if it really makes sense to pass env_without_make to that call to make itself -- I mean, if you explicitly invoke "make", setting $MAKE to something else doesn't seem to have any effect.
Comment 4 Gustavo Noronha (kov) 2012-03-26 09:02:51 PDT
That's a good point. If it still works if you set MAKE='make -j4' and remove that, then sure, it doesn't hurt to have it though.
Comment 5 Raphael Kubo da Costa (:rakuco) 2012-03-26 13:03:44 PDT
Created attachment 133872 [details]
Patch
Comment 6 Raphael Kubo da Costa (:rakuco) 2012-03-26 13:04:38 PDT
(In reply to comment #4)
> That's a good point. If it still works if you set MAKE='make -j4' and remove that, then sure, it doesn't hurt to have it though.

It does, and r112139 removed the os.environ workaround which was in place.
Comment 7 Martin Robinson 2012-03-26 13:11:12 PDT
Comment on attachment 133872 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=133872&action=review

> Tools/jhbuild/jhbuild-wrapper:96
> +    make = shlex.split(os.environ.get('MAKE', 'make'))

Why not just os.environ.get('MAKE', 'make').split()[0] ?
Comment 8 Raphael Kubo da Costa (:rakuco) 2012-03-26 13:12:40 PDT
(In reply to comment #7)
> (From update of attachment 133872 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=133872&action=review
> 
> > Tools/jhbuild/jhbuild-wrapper:96
> > +    make = shlex.split(os.environ.get('MAKE', 'make'))
> 
> Why not just os.environ.get('MAKE', 'make').split()[0] ?

If MAKE is set to "make -j3" that will only give me "make" and "-j3" will be lost. shlex.split("make -j3") gives me ['make', '-j3'].
Comment 9 Martin Robinson 2012-03-26 13:25:07 PDT
(In reply to comment #8)

> If MAKE is set to "make -j3" that will only give me "make" and "-j3" will be lost. shlex.split("make -j3") gives me ['make', '-j3'].

In that case, why not os.environ.get('MAKE', 'make').split()?

>>> "make -j4".split()
['make', '-j4']
Comment 10 Raphael Kubo da Costa (:rakuco) 2012-03-26 13:29:05 PDT
(In reply to comment #9)
> (In reply to comment #8)
> 
> > If MAKE is set to "make -j3" that will only give me "make" and "-j3" will be lost. shlex.split("make -j3") gives me ['make', '-j3'].
> 
> In that case, why not os.environ.get('MAKE', 'make').split()?
> 
> >>> "make -j4".split()
> ['make', '-j4']

Mostly because subprocess.Popen's documentation recommends using shlex.split() for proper argument tokenization. I can move to just using str.split() if that's preferred, though.
Comment 11 Martin Robinson 2012-03-26 14:41:10 PDT
Comment on attachment 133872 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=133872&action=review

>>> Tools/jhbuild/jhbuild-wrapper:96
>>> +    make = shlex.split(os.environ.get('MAKE', 'make'))
>> 
>> Why not just os.environ.get('MAKE', 'make').split()[0] ?
> 
> If MAKE is set to "make -j3" that will only give me "make" and "-j3" will be lost. shlex.split("make -j3") gives me ['make', '-j3'].

Makes sense, thanks!
Comment 12 Raphael Kubo da Costa (:rakuco) 2012-03-26 15:15:26 PDT
Committed r112153: <http://trac.webkit.org/changeset/112153>