added QNX SPD CLI support#57
Conversation
srinivasugithub
requested review from
AlexanderLanin,
antonkri,
dcalavrezo-qorix and
nradakovic
as code owners
| @@ -0,0 +1,26 @@ | |||
| # ******************************************************************************* | |||
There was a problem hiding this comment.
Why did you create module here? As far as I can see, we don't need this.
| ) | ||
|
|
||
| # Package the SDP as a tarball for distribution (optional) | ||
| genrule( |
There was a problem hiding this comment.
We don't need genrule to package the output of CLI QNX (SDP). Bazel support already all sorts of packaging rules like pkg_tar or pkg_zip.
| ) | ||
|
|
||
| # Create a filegroup that references the output directory | ||
| filegroup( |
|
|
||
| """Rule to create a QNX SDP distribution from a patchset file.""" | ||
|
|
||
| def _qnx_sdp_distribution_impl(ctx): |
There was a problem hiding this comment.
why we need to run the command over the script? Why not directly via ctx.actuins.run?
There was a problem hiding this comment.
I didn't see at start what is done here. The script that is here, I would set it as resource and just pull it into the rule as tool dependency.
| # "gcc_version": "12.2.0", | ||
| # }, | ||
| "aarch64-qnx-sdp_8.0.0": { | ||
| "build_file": "@score_bazel_cpp_toolchains//packages/qnx/aarch64/sdp/8.0.0:sdp.BUILD", |
There was a problem hiding this comment.
Please do not remove old references, just add the new one if you need them.
| # Create a BUILD file that exports the qnxsoftwarecenter_clt binary | ||
| # Note: The installer extracts to ./qnxsoftwarecenter (not qnx_software_center/qnxsoftwarecenter) | ||
| repository_ctx.file("BUILD", """ | ||
| exports_files([ |
There was a problem hiding this comment.
You do not need this. Filegroup already exposes tool.
| doc = "The QNX installer file", | ||
| ), | ||
| }, | ||
| local = False, |
There was a problem hiding this comment.
isn't this option by default set?
| @@ -0,0 +1,49 @@ | |||
| # ******************************************************************************* | |||
There was a problem hiding this comment.
rename SDP directory to sdp_install.
| @@ -0,0 +1,38 @@ | |||
| #!/bin/bash | |||
There was a problem hiding this comment.
if you want to have some sanity testing in place, then create directory test under sdp_install and set all test files and resources there.
|
|
||
| # Create a BUILD file that exports the qnxsoftwarecenter_clt binary | ||
| # Note: The installer extracts to ./qnxsoftwarecenter (not qnx_software_center/qnxsoftwarecenter) | ||
| repository_ctx.file("BUILD", """ |
There was a problem hiding this comment.
I would consider having BUILD file set as resource rather than writing it within the rule.
| progress_message = "Installing QNX SDP from patchset %s" % patchset.short_path, | ||
| use_default_shell_env = True, | ||
| execution_requirements = { | ||
| "no-sandbox": "1", # May need to run without sandbox for installer |
There was a problem hiding this comment.
This is very bad option. It will destroy hermeticity of the build.
|
|
||
| """Rule to create a QNX SDP distribution from a patchset file.""" | ||
|
|
||
| def _qnx_sdp_distribution_impl(ctx): |
There was a problem hiding this comment.
I didn't see at start what is done here. The script that is here, I would set it as resource and just pull it into the rule as tool dependency.
2 participants
added QNX SPD CLI support