-
Notifications
You must be signed in to change notification settings - Fork 696
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
ci(build-and-test-differential): remove sync for the build-and-test-differential workflows #5200
Conversation
a5436d3
to
210a61e
Compare
…ifferential workflows Signed-off-by: M. Fatih Cırıt <mfc@leodrive.ai>
210a61e
to
4cce03f
Compare
I agree that sync file job has become too complex. I was just wondering if you want to find out a simpler way of maintaining similar files across different repositories, or you rather want to maintain them manually. |
I think we can maintain just the Universe one manually for now. Once we merge this, there are 3 potential futures:
I'd like to go with option 1 for now since it requires the least effort. Additional note on "self-hosted" naming scheme in many workflow files:The Until now, we had all the arm things built in self hosted instances. But now we build x86 things in self hosted instances too. But this will be so many refactorings all over the place, it is not high priority. |
@xmfcx Thanks. For the naming convention for "-self-hosted", you are right that we should probably rename them to "-arm", but I agree with you that it is low priority for now. |
* fix devcontainer naming Signed-off-by: Oguz Ozturk <oguzkaganozt@gmail.com> * Update .devcontainer/cuda/devcontainer.json Co-authored-by: Yutaka Kondo <yutaka.kondo@youtalk.jp> * Update .devcontainer/base/devcontainer.json Co-authored-by: Yutaka Kondo <yutaka.kondo@youtalk.jp> --------- Signed-off-by: Oguz Ozturk <oguzkaganozt@gmail.com> Co-authored-by: Yutaka Kondo <yutaka.kondo@youtalk.jp>
Description
Directly related sync job:
Proposal
To me, with the latest changes introduced by us, the sync job has grown way too much in complexity to be easily understood by anyone.
I think it is time to cut off the sync job at least for the
build-and-test-differential.yaml
workflow.At least until we find a better way to sync this job. Or maybe once we make the containers smaller, we can remove the self hosted option too.
Function of this sync job
It syncs from:
to:
But while syncing, it makes some brittle changes to the original file as instructed in the sync job file:
ChatGPT analysis of the current sync job
According to ChatGPT analysis, the rename operations can be summarized as follows:
The
pre-commands
in the second YAML file are usingsd
(substitute) command, which is a tool used for substituting or replacing strings in files, somewhat similar tosed
command in Unix.First Command:
It substitutes all occurrences of
container: ros:<any_word_character>
withcontainer: ghcr.io/autowarefoundation/autoware-universe:<any_word_character>-latest
.Here
<any_word_character>
is captured by(\w+)
and replaced by\$1
.This change is applied to the
{source}
, referring to.github/workflows/build-and-test-differential.yaml
.Second Command:
It substitutes the first occurrence of
container: ${{ matrix.container }}
withcontainer: ${{ matrix.container }}${{ matrix.container-suffix }}
in{source}
, adding a potential suffix to the container matrix.Third Command:
This substitutes
"include:"
with a block that introduces a new matrix variablecontainer-suffix
with two values, an empty string, and-cuda
, followed by the originalinclude:
.This likely enables running the workflow with and without CUDA support by appending the suffix to the container name.
Fourth Command:
It substitutes all occurrences of
container: ghcr.io/autowarefoundation/autoware-universe:<any_word_character>-latest
at the beginning of a line withcontainer: ghcr.io/autowarefoundation/autoware-universe:<any_word_character>-latest-cuda
,effectively appending
-cuda
to the end of the container names.In Summary:
These
pre-commands
are modifying thebuild-and-test-differential.yaml
workflow's container names and introducing a new matrix variable,likely to adjust the environment in which the jobs are run,
possibly to add support for CUDA and to use specific container images from
ghcr.io/autowarefoundation/autoware-universe
.History of the sync job
Here is a brief history of the
build-and-test-differential
sync job in the Universe repository:And the manual changes made by me and @mitsudome-r :
Tests performed
Not applicable.
Effects on system behavior
Not applicable.
Pre-review checklist for the PR author
The PR author must check the checkboxes below when creating the PR.
In-review checklist for the PR reviewers
The PR reviewers must check the checkboxes below before approval.
Post-review checklist for the PR author
The PR author must check the checkboxes below before merging.
After all checkboxes are checked, anyone who has write access can merge the PR.