Add support for building clang targeted for CentOS6 build machines

RESOLVED FIXED in Firefox 43

Status

defect
RESOLVED FIXED
4 years ago
Last year

People

(Reporter: Ehsan, Unassigned)

Tracking

unspecified
mozilla43
Dependency tree / graph

Firefox Tracking Flags

(firefox43 fixed)

Details

Attachments

(1 attachment, 2 obsolete attachments)

No description provided.
Blocks: 1203390
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.
Attachment #8659043 - Flags: review?(rail) → review-
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.
Attachment #8659046 - Flags: review?(rail) → review-
(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?
Flags: needinfo?(rail)
Sure. Just fix the "python_path" issue in this case, it's not used anywhere.
Flags: needinfo?(rail)
Attachment #8659043 - Attachment is obsolete: true
Attachment #8659046 - Attachment is obsolete: true
Attachment #8659558 - Flags: review?(rail)
Attachment #8659558 - Flags: review?(rail) → review+
https://hg.mozilla.org/mozilla-central/rev/0f88a6861417
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.