Closed Bug 1467897 Opened 7 years ago Closed 7 years ago

Support local executions of build-clang

Categories

(Firefox Build System :: Toolchains, enhancement)

3 Branch
enhancement
Not set
normal

Tracking

(firefox63 fixed)

RESOLVED FIXED
mozilla63
Tracking Status
firefox63 --- fixed

People

(Reporter: tjr, Assigned: tjr)

Details

Attachments

(1 file)

Currently, build-clang.py hardcodes a path that's used in our Docker workers. If we make that path configurable; then users who are patching the compiler can run build-clang.py locally (if they specify a json file that uses local paths.)
Comment on attachment 8984577 [details] Bug 1467897 Allow local runs of build-clang by providing a --base-dir option https://reviewboard.mozilla.org/r/250450/#review256748 ::: build/build-clang/build-clang.py:347 (Diff revision 1) > + if args.base_dir: > + base_dir = args.base_dir This has no effect on windows because of the if is_windows() block below. But actually, the is_windows() case seems about the right default for what to do locally, just ensure the directory doesn't exist first before doing anything that might overwrite existing files. So, here's what I think would be nicer: - when MOZ_AUTOMATION is set, default to '/builds/worker/....' on `not is_windows()`. - otherwise, default to cwd + llvm-sources, or maybe change llvm-sources to build-clang or something. - check that the directory that is picked does not exist. - If it does, error out and tell the user to pass a base dir on the command line.
Attachment #8984577 - Flags: review?(mh+mozilla)
(In reply to Mike Hommey [:glandium] from comment #2) > just ensure the directory doesn't exist first before doing > anything that might overwrite existing files. > > So, here's what I think would be nicer: > - when MOZ_AUTOMATION is set, default to '/builds/worker/....' on `not > is_windows()`. > - otherwise, default to cwd + llvm-sources, or maybe change llvm-sources to > build-clang or something. > - check that the directory that is picked does not exist. > - If it does, error out and tell the user to pass a base dir on the command > line. Hm. So we want to allow the user to work with existing directories; so they don't need to re-checkout the source trees everytime. So we can't just error if the existing directory exists at all. I changed it around to drop a .build-clang and use that as an indicator if we can reuse the directory.
Comment on attachment 8984577 [details] Bug 1467897 Allow local runs of build-clang by providing a --base-dir option https://reviewboard.mozilla.org/r/250450/#review260710 Code analysis found 1 defect in this patch: - 1 defect found by mozlint You can run this analysis locally with: - `./mach lint path/to/file` (JS/Python) If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx ::: build/build-clang/build-clang.py:374 (Diff revision 2) > + os.sys.exit(0) > + > + if not os.path.exists(base_dir): > + os.makedirs(base_dir) > + elif os.listdir(base_dir) and '.build-clang' not in os.listdir(base_dir): > + raise ValueError("Base directory %s exists and is not a build-clang directory. " \ Error: The backslash is redundant between brackets [flake8: E502]
Comment on attachment 8984577 [details] Bug 1467897 Allow local runs of build-clang by providing a --base-dir option https://reviewboard.mozilla.org/r/250450/#review261516 ::: build/build-clang/build-clang.py:333 (Diff revision 4) > + parser = argparse.ArgumentParser() > + parser.add_argument('-c', '--config', required=True, > + type=argparse.FileType('r'), > + help="Clang configuration file") > + parser.add_argument('-b', '--base-dir', required=False, > + type=str, type=str shouldn't be necessary. ::: build/build-clang/build-clang.py:369 (Diff revision 4) > source_dir = base_dir + "/src" > build_dir = base_dir + "/build" > > + if not os.path.exists(base_dir): > + os.makedirs(base_dir) > + elif os.listdir(base_dir) and '.build-clang' not in os.listdir(base_dir): os.path.exists(os.path.join(base_dir, '.build-clang'))
Attachment #8984577 - Flags: review?(mh+mozilla) → review+
Keywords: checkin-needed
Pushed by rgurzau@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/1cbc11323303 Allow local runs of build-clang by providing a --base-dir option r=glandium
Keywords: checkin-needed
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Version: Version 3 → 3 Branch
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: