-
-
Notifications
You must be signed in to change notification settings - Fork 21
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for using artifacts from another Job #185
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #185 +/- ##
==========================================
+ Coverage 74.69% 74.76% +0.07%
==========================================
Files 51 51
Lines 5865 5913 +48
==========================================
+ Hits 4381 4421 +40
- Misses 1484 1492 +8 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
c8b3f84
to
bbb0767
Compare
README.md
Outdated
@@ -859,3 +859,34 @@ For the `fetch` stage, the Qubes executor with disposable template `qubes-builde | |||
For the `build` stage of `vm-fc42`, the Podman executor with container image `fedoraimg` will be used. | |||
For the `sign` stage, the Qubes executor with disposable template `signing-access-dvm` will be used for both `vm-fc42` and `vm-jammy` | |||
For the `prep` stage of `vm-jammy`, the Local executor with base directory `/some/path` will be used. | |||
|
|||
### Cross-compile Build |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Technically, cross-compile is something else, like building ARM binaries on X86. Better use different term, like cross-distribution dependencies
README.md
Outdated
|
||
```yaml | ||
components: | ||
- installer-qubes-os: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
installer-qubes-os-windows-tools? (and below too)
qubesbuilder/config.py
Outdated
if ( | ||
isinstance(stage, dict) | ||
and next(iter(stage)) == stage_name | ||
and isinstance(stage[stage_name], dict) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At least some of this not matching should show a warning or even error. Otherwise it will be hard to find why it doesn't work if you make a typo (like extra -
making it a list instead of dict).
if all( | ||
[ | ||
need.get("component", None), | ||
need.get("distribution", None), | ||
need.get("stage", None), | ||
need.get("build", None), | ||
] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and same here - if some part is missing, at least log a warning why it's ignored
qubesbuilder/plugins/__init__.py
Outdated
copy_in.append( | ||
(artifact, self.executor.get_dependencies_dir()) | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should they all be copied together, or maybe better in separate subdirs? what if file names conflict (for example you declare QWT use vm-win10 and vm-win11 if that would exist)?
9593265
to
9c76892
Compare
Our usecase is for cross-compiling Windows tools
4552148
to
39e023f
Compare
From CI logs:
Besides the error itself, I'd prefer to not use absolute paths in artifacts info. Specifically, relative to the relevant artifacts dir. So, in this example it would be just
|
I don't see where is it supposed to write absolute path, indeed this is not intended. |
It allows to know what has been created by jobs.
Ok it should be good for review. |
Maybe I should add yml checks of |
Our usecase is for cross-compiling Windows tools