Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Sync develop to master #25

Merged
merged 5 commits into from
Apr 22, 2024
Merged

Sync develop to master #25

merged 5 commits into from
Apr 22, 2024
+20 −19

Conversation

paulbourelly999
Copy link
Contributor

PR Details

Description

This is an intermediate branch cut from master which includes fixes to the branch coupling of build and checkout scripts. This coupling can not yet be removed from CI processes because this repository still uses CircleCI. The functionality to decouple CI from branches is currently only available through our GitHub actions

Related GitHub Issue

Related Jira Key

Motivation and Context

Decouple build/checkout from branch to make release process simpler

How Has This Been Tested?

CI

Types of changes

  • Defect fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that cause existing functionality to change)

Checklist:

  • I have added any new packages to the sonar-scanner.properties file
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@SaikrishnaBairamoni
update configs to point candidate images
4466cf1
@paulbourelly999
Update version numbers to 4.5.0 (#23)
ce2570f 
<!-- Thanks for the contribution, this is awesome. -->

# PR Details
## Description

<!--- Describe your changes in detail -->

## Related GitHub Issue

<!--- This project only accepts pull requests related to open GitHub
issues or Jira Keys -->
<!--- If suggesting a new feature or change, please discuss it in an
issue first -->
<!--- If fixing a bug, there should be an issue describing it with steps
to reproduce -->
<!--- Please DO NOT name partially fixed issues, instead open an issue
specific to this fix -->
<!--- Please link to the issue here: -->

## Related Jira Key

<!-- e.g. CAR-123 -->

## Motivation and Context

<!--- Why is this change required? What problem does it solve? -->

## How Has This Been Tested?

<!--- Please describe in detail how you tested your changes. -->
<!--- Include details of your testing environment, and the tests you ran
to -->
<!--- see how your change affects other areas of the code, etc. -->

## Types of changes

<!--- What types of changes does your code introduce? Put an `x` in all
the boxes that apply: -->

- [ ] Defect fix (non-breaking change that fixes an issue)
- [ ] New feature (non-breaking change that adds functionality)
- [ ] Breaking change (fix or feature that cause existing functionality
to change)

## Checklist:

<!--- Go over all the following points, and put an `x` in all the boxes
that apply. -->
<!--- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->

- [ ] I have added any new packages to the sonar-scanner.properties file
- [ ] My change requires a change to the documentation.
- [ ] I have updated the documentation accordingly.
- [ ] I have read the
[**CONTRIBUTING**](https://github.com/usdot-fhwa-stol/carma-platform/blob/develop/Contributing.md)
document.
- [ ] I have added tests to cover my changes.
- [ ] All new and existing tests passed.
@MishkaMN
@MishkaMN
Merge release/lavida (4.5.0) branch into master (#22)
0cbe800 
<!-- Thanks for the contribution, this is awesome. -->

# PR Details
## Description
**Requires closing #23**
<!--- Describe your changes in detail -->

## Related GitHub Issue

<!--- This project only accepts pull requests related to open GitHub
issues or Jira Keys -->
<!--- If suggesting a new feature or change, please discuss it in an
issue first -->
<!--- If fixing a bug, there should be an issue describing it with steps
to reproduce -->
<!--- Please DO NOT name partially fixed issues, instead open an issue
specific to this fix -->
<!--- Please link to the issue here: -->

## Related Jira Key

<!-- e.g. CAR-123 -->

## Motivation and Context

<!--- Why is this change required? What problem does it solve? -->

## How Has This Been Tested?

<!--- Please describe in detail how you tested your changes. -->
<!--- Include details of your testing environment, and the tests you ran
to -->
<!--- see how your change affects other areas of the code, etc. -->

## Types of changes

<!--- What types of changes does your code introduce? Put an `x` in all
the boxes that apply: -->

- [ ] Defect fix (non-breaking change that fixes an issue)
- [ ] New feature (non-breaking change that adds functionality)
- [ ] Breaking change (fix or feature that cause existing functionality
to change)

## Checklist:

<!--- Go over all the following points, and put an `x` in all the boxes
that apply. -->
<!--- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->

- [ ] I have added any new packages to the sonar-scanner.properties file
- [ ] My change requires a change to the documentation.
- [ ] I have updated the documentation accordingly.
- [ ] I have read the
[**CONTRIBUTING**](https://github.com/usdot-fhwa-stol/carma-platform/blob/develop/Contributing.md)
document.
- [ ] I have added tests to cover my changes.
- [ ] All new and existing tests passed.
@MishkaMN
@paulbourelly999
Update build and checkout to remove branch coupling
b332bdc
@paulbourelly999 paulbourelly999 self-assigned this Apr 19, 2024
@paulbourelly999 paulbourelly999 mentioned this pull request Apr 19, 2024
Closed
6 tasks
adamlm
adamlm previously requested changes Apr 19, 2024
View reviewed changes
docker/checkout.bash Outdated
Comment on lines 40 to 46
sudo git clone https://github.com/usdot-fhwa-stol/carma-msgs.git ${dir}/src/CARMAMsgs --branch "$BRANCH"
sudo git clone https://github.com/usdot-fhwa-stol/carma-utils.git ${dir}/src/CARMAUtils --branch "$BRANCH"
sudo git clone https://github.com/usdot-fhwa-stol/autoware.ai.git ${dir}/src/autoware.ai --branch carma-"$BRANCH"
else
sudo git clone https://github.com/usdot-fhwa-stol/carma-msgs.git ${dir}/src/CARMAMsgs --branch develop
sudo git clone https://github.com/usdot-fhwa-stol/carma-utils.git ${dir}/src/CARMAUtils --branch develop
sudo git clone https://github.com/usdot-fhwa-stol/autoware.ai.git ${dir}/src/autoware.ai --branch carma-develop
sudo git clone https://github.com/usdot-fhwa-stol/carma-msgs.git ${dir}/src/CARMAMsgs --branch "$BRANCH"
sudo git clone https://github.com/usdot-fhwa-stol/carma-utils.git ${dir}/src/CARMAUtils --branch "$BRANCH"
sudo git clone https://github.com/usdot-fhwa-stol/autoware.ai.git ${dir}/src/autoware.ai --branch "$BRANCH"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like carma-msgs and carma-utils repos can be moved out of the conditional.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolved.

@paulbourelly999 paulbourelly999 requested review from adamlm and removed request for adamlm April 19, 2024 19:02
@paulbourelly999 paulbourelly999 dismissed adamlm’s stale review April 19, 2024 19:49

Adam is leaving STOL

@adev4a
Copy link

adev4a commented Apr 19, 2024 •
edited
Loading

It looks like the build is failing on the CI step but succeeding as a docker build, which seems to point to something in the install script that's missing from the setup in the CI script

@paulbourelly999
Copy link
Contributor Author

It looks like the build is failing on the CI step but succeeding as a docker build, which seems to point to something in the install script that's missing from the setup in the CI script

@adev4a The circle ci workflow is failing on a unit test of the ndt_cpu package. I am not sure if this package is even part of this repo or part of a dependency. Regardless this CI has never executed successfully. I would argue therefore that this is an unrelated bug in the build process that needs to be addressed separately

MishkaMN
MishkaMN approved these changes Apr 22, 2024
View reviewed changes
.circleci/config.yml Show resolved Hide resolved
docker/checkout.bash
fi

#TODO This probably needs to be moved into install.sh
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NIT: We should probably create github or JIRA ticket (in devops backlog) to properly track this

@paulbourelly999 paulbourelly999 merged commit 3450f94 into develop Apr 22, 2024
1 of 2 checks passed
@paulbourelly999 paulbourelly999 deleted the sync-develop-to-master branch April 22, 2024 15:45
JonSmet
JonSmet reviewed Apr 23, 2024
View reviewed changes
Dockerfile

COPY --chown=carma . /home/carma/autoware.auto
RUN chmod -R 775 /home/carma/autoware.auto/docker/checkout.bash
RUN chmod -R 775 /home/carma/autoware.auto/docker/install.sh
RUN /home/carma/autoware.auto/docker/checkout.bash
RUN /home/carma/autoware.auto/docker/checkout.bash -b ${GIT_BRANCH}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since users will typically use the build-image.sh script directly, which calls this Dockerfile, do we want to update build-image.sh to take a git branch as an argument, and then pass it as a GIT_BRANCH arg into Dockerfile?

Though, less of a priority since we don't build an Autoware.Auto image (instead, this repo is checked out during the autoware.ai build) for usage in CARMA Platform.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JonSmet JonSmet JonSmet left review comments

@MishkaMN MishkaMN MishkaMN approved these changes

@adamlm adamlm adamlm left review comments

@adev4a adev4a Awaiting requested review from adev4a

@SaikrishnaBairamoni SaikrishnaBairamoni Awaiting requested review from SaikrishnaBairamoni

Assignees

@paulbourelly999 paulbourelly999

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@paulbourelly999 @adev4a @MishkaMN @adamlm @JonSmet @SaikrishnaBairamoni