Dependency targets #250
Dependency targets #250
Conversation
fischeti
left a comment
fischeti
left a comment
There was a problem hiding this comment.
LGTM! The dependency targest are definitely nice and also the way passed targets are handled looks a bit cleaner now.
| Version(TargetSpec, semver::VersionReq, Vec<PassedTarget>), | ||
| /// A local path dependency. The exact version of the dependency found at | ||
| /// the given path will be used, regardless of any actual versioning | ||
| /// constraints. | ||
| Path(PathBuf, Vec<String>), | ||
| Path(TargetSpec, PathBuf, Vec<PassedTarget>), | ||
| /// A git dependency specified by a revision. | ||
| GitRevision(String, String, Vec<String>), | ||
| GitRevision(TargetSpec, String, String, Vec<PassedTarget>), | ||
| /// A git dependency specified by a version requirement. Works similarly to | ||
| /// the `GitRevision`, but extracts all tags of the form `v.*` from the | ||
| /// repository and matches the version against that. | ||
| GitVersion(String, semver::VersionReq, Vec<String>), | ||
| GitVersion(TargetSpec, String, semver::VersionReq, Vec<PassedTarget>), |
There was a problem hiding this comment.
With that many fields, it could potentially make sense to change them to named fields now to make it clear what they stand for.
|
|
||
| # Git revision dependency. | ||
| common_cells: { git: "git@github.com:pulp-platform/common_cells.git", rev: master, pass_targets: ["CC_CUSTOM_TARGET"] } | ||
| common_cells: { target: test, git: "git@github.com:pulp-platform/common_cells.git", rev: master, pass_targets: ["CC_CUSTOM_TARGET", {target: "any(*, test)", pass: "cc_test"}] } |
There was a problem hiding this comment.
I would separate the target examples here since it is a bit overloaded. Then, for each type of target you can explain in the comment above what it does. For instance:
# Using a dependency only if (global?) target `test` is used
common_verification: {git: "git@github.com:pulp-platform/common_verification.git", version: 0.1.0, target: "test"}
# Passing a target to a dependency (equivalent to `-t cva6:cv64a6_imafdcv_sv39`)
cva6: {git: "git@github.com:pulp-platform/cva6.git", version: 0.1.0, pass_targets: "cv64a6_imafdcv_sv39"}
# Remapping a local target if a global target is passed
tech_cells_generic:
- git: git@github.com:pulp-platform/tech_cells_generic.git
- version: 0.1.0
- pass_targets:
- target: all(asic, tech7)
- pass: tech7_macro # Maybe you have a better exampleThere was a problem hiding this comment.
Also why is CC_CUSTOM_TARGET uppercase? It is kind of confusing since it gives the impression that this target should be upper case, but targets are case insensitive. Lowercase targets look better, and I would not want people to start using uppercase targets (even if it does not matter).
No description provided.