Closed Bug 1329307 Opened 3 years ago Closed 3 years ago

Only package the clang-tidy binaries for the clang-tidy builds

Categories

(Firefox Build System :: Source Code Analysis, defect)

defect
Not set

Tracking

(firefox53 fixed)

RESOLVED FIXED
mozilla53
Tracking Status
firefox53 --- fixed

People

(Reporter: ehsan, Assigned: ehsan)

References

Details

Attachments

(1 file)

No description provided.
This reduced the download size for bug 1328454 by an order of magnitude.
Assignee: nobody → ehsan
Blocks: 1328454
Comment on attachment 8824528 [details] [diff] [review]
Only package the clang-tidy binaries for the clang-tidy builds

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

::: build/build-clang/build-clang.py
@@ +133,5 @@
> +        shutil.rmtree(path)
> +    # exists() returns false for broken symbolic links, so we need to allow
> +    # them explicitly.
> +    elif os.path.exists(path) or os.path.islink(path):
> +        os.unlink(path)

I'm pretty sure that the "pythonic" way to write this would be something like:

try:
    os.unlink(path)
except:
    pass

@@ +290,5 @@
>      except which.WhichError:
>          raise ValueError("%s not found on PATH" % f)
>  
> +
> +def prune_final_dir_for_clang_tidy(final_dir):

Can we have a doc comment on this function? I understand the goals of this function but I imagine that just the name may not be enough to make it clear in the future.

@@ +591,5 @@
>              llvm_source_dir, stage3_dir, build_libcxx, osx_cross_compile,
>              build_type, assertions, python_path, gcc_dir)
>  
> +    package_name = "clang"
> +    if import_clang_tidy:

I feel like "import_clang_tidy" is a poor name now that the artifacts which are produced don't actually contain "clang + mozilla clang tidy extensions" but contain _only clang tidy_.

Perhaps build_clang_tidy would be better?
Attachment #8824528 - Flags: review?(michael) → review+
Pushed by eakhgari@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c05634d71a73
Only package the clang-tidy binaries for the clang-tidy builds; r=mystor
https://hg.mozilla.org/mozilla-central/rev/c05634d71a73
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.