1203388 - Add support for building clang targeted for CentOS6 build machines

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Comment 1

[(no longer active)]

Attachment: Add support for building clang targeted for CentOS6 build machines

Comment 2

[(no longer active)]

Attachment: Add support for building clang targeted for CentOS6 build machines

Comment 3

[Rail Aliiev [:rail]]

Comment on attachment 8659043 [details] [diff] [review]
Add support for building clang targeted for CentOS6 build machines

Review of attachment 8659043 [details] [diff] [review]:
-----------------------------------------------------------------

This would require yiu to change the script depending on your OS. How about moving the paths to configs? Maybe something like this: https://gist.github.com/rail/550db1c4936af29ab6c0. You'd need to create a config per OS in this case.

::: build/unix/build-clang/build-clang.py
@@ +98,5 @@
>  
> +    if centOS6:
> +        python_path = "/usr/bin/python2.7"
> +    else:
> +        python_path = "/usr/local/bin/python2.7"

The variable is set but unused.

Comment 4

[Rail Aliiev [:rail]]

Comment on attachment 8659046 [details] [diff] [review]
Add support for building clang targeted for CentOS6 build machines

Review of attachment 8659046 [details] [diff] [review]:
-----------------------------------------------------------------

Use the same approach as for the previous patch?

::: build/unix/build-clang/build-clang.py
@@ +99,5 @@
> +    global centOS6
> +    if centOS6:
> +        python_path = "/usr/bin/python2.7"
> +    else:
> +        python_path = "/usr/local/bin/python2.7"

python_path is unused here.

Comment 5

[(no longer active)]

(In reply to Rail Aliiev [:rail] from comment #3)
> Comment on attachment 8659043 [details] [diff] [review]
> Add support for building clang targeted for CentOS6 build machines
> 
> Review of attachment 8659043 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> This would require yiu to change the script depending on your OS.

I have a stack of patches (currently 8) for bug 1182727 which I haven't submitted yet that would clean up this script and would move all sorts of stuff like this to be declared in the config file.  The reason why I haven't submitted them yet is that they run into build issues on the old CentOS machines and I haven't had time to figure it out yet.  While your idea is nice, doing it now means that I will need to rebase all of those patches and that's difficult, so I prefer to do the less optimal thing now and move it to the config files later in that bug.  Is that OK with you?

Comment 6

[Rail Aliiev [:rail]]

Sure. Just fix the "python_path" issue in this case, it's not used anywhere.

Comment 7

[(no longer active)]

Attachment: Add support for building clang targeted for CentOS6 build machines

Comment 8

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/0f88a6861417

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/0f88a6861417