Page MenuHomePhabricator
Differential D5628

Use ninja to generate dep files for the native build
ClosedPublic
Actions

Authored by deadalnix on Mar 31 2020, 19:23.

Details

Reviewers
Fabien
Group Reviewers
Restricted Project
Commits
rSTAGINGec66b9b27e2e: Use ninja to generate dep files for the native build
rABCec66b9b27e2e: Use ninja to generate dep files for the native build
Summary

This introduce a python script that is able to generate dep files from ninja.
The script is used to generate a proper dep file for re-running cmake for the native build, but it is generic and can be used for the native targets in the future.

Depends on D5624

Test Plan
ninja
ninja # Check native build isn't ran again.
touch build.ninja
ninja # Native build runs again.

Diff Detail

Repository
rABC Bitcoin ABC
Branch
fullnativedeps
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 10076
Build 17985: Default Diff Build & Tests
Build 17984: arc lint + arc unit

Event Timeline

deadalnix created this revision.Mar 31 2020, 19:23
Herald added a reviewer: Restricted Project. · View Herald TranscriptMar 31 2020, 19:23
teamcity added a comment.Mar 31 2020, 19:23

Build Bitcoin-ABC / Diffs / Diff Testing (build-diff) started.

teamcity added a comment.Mar 31 2020, 19:32

Build Bitcoin-ABC / Diffs / Diff Testing (build-diff) passed.

Harbormaster completed remote builds in B10063: Diff 17333.Mar 31 2020, 19:32
deadalnix updated this revision to Diff 17334.Mar 31 2020, 20:19

remove leftover comment

teamcity added a comment.Mar 31 2020, 20:19

Build Bitcoin-ABC / Diffs / Diff Testing (build-diff) started.

teamcity added a comment.Mar 31 2020, 20:28

Build Bitcoin-ABC / Diffs / Diff Testing (build-diff) passed.

Harbormaster completed remote builds in B10064: Diff 17334.Mar 31 2020, 20:28
deadalnix updated this revision to Diff 17335.Mar 31 2020, 20:42

Use something simpler for extra dependencies.

teamcity added a comment.Mar 31 2020, 20:42

Build Bitcoin-ABC / Diffs / Diff Testing (build-diff) started.

teamcity added a comment.Mar 31 2020, 20:51

Build Bitcoin-ABC / Diffs / Diff Testing (build-diff) passed.

Harbormaster completed remote builds in B10065: Diff 17335.Mar 31 2020, 20:51
Fabien requested changes to this revision.Apr 1 2020, 08:51
Fabien added a subscriber: Fabien.

If you have a document for the depfile format, please link it somewhere.

A few nits, but overall LGTM. I tested with various cmake and ninja versions, as well as OSX.

cmake/utils/gen-ninja-deps.py
39 ↗(On Diff #17335)

Use command -v instead, which is not portable.

77 ↗(On Diff #17335)

Are they guaranteed to be sorted that way ? If not you want to continue on order dependency instead of breaking

94 ↗(On Diff #17335)

Remove the blank lines

136 ↗(On Diff #17335)

Move to rebase_deps

This revision now requires changes to proceed.Apr 1 2020, 08:51
deadalnix added inline comments.Apr 1 2020, 12:36
cmake/utils/gen-ninja-deps.py
94 ↗(On Diff #17335)

This is python's style.

Fabien added inline comments.Apr 1 2020, 12:40
cmake/utils/gen-ninja-deps.py
94 ↗(On Diff #17335)

The consider using docstring instead. Having a floating comment like this makes no sense.

deadalnix added inline comments.Apr 1 2020, 14:18
cmake/utils/gen-ninja-deps.py
77 ↗(On Diff #17335)

yes

deadalnix updated this revision to Diff 17354.Apr 1 2020, 14:18

Address comments

teamcity added a comment.Apr 1 2020, 14:18

Build Bitcoin-ABC / Diffs / Diff Testing (build-diff) started.

teamcity added a comment.Apr 1 2020, 14:27

Build Bitcoin-ABC / Diffs / Diff Testing (build-diff) passed.

Harbormaster completed remote builds in B10076: Diff 17354.Apr 1 2020, 14:28
Fabien accepted this revision.Apr 1 2020, 14:38
Fabien added inline comments.
cmake/utils/gen-ninja-deps.py
135

This one is still there, but should be local to rebase_deps since it's the only place it is used.

This revision is now accepted and ready to land.Apr 1 2020, 14:38
deadalnix added inline comments.Apr 1 2020, 14:56
cmake/utils/gen-ninja-deps.py
135

It's a command line parameter, so it's obviously not scoped to that function. The fact that it happens to be used only there is simply an implementation detail.

Closed by commit rABCec66b9b27e2e: Use ninja to generate dep files for the native build (authored by deadalnix). · Explain WhyApr 1 2020, 14:56
This revision was automatically updated to reflect the committed changes.
deadalnix added a commit: rABCec66b9b27e2e: Use ninja to generate dep files for the native build.
deadalnix added a commit: rSTAGINGec66b9b27e2e: Use ninja to generate dep files for the native build.Apr 7 2020, 23:34

Revision Contents
Changeset List

PathSize
cmake/
templates/
11 lines
utils/
181 lines

Diff 17354

View Options

cmake/templates/NativeCmakeRunner.cmake.in

Loading...
View Options

cmake/utils/gen-ninja-deps.py

Loading...