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.

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

Fix CI and Makefiles to handle dockerfile submodule dependencies. #1

Open
wants to merge 1 commit into
base: sai-c-refactoring
Choose a base branch
from

Conversation

chrispsommers
Copy link

See the changes which resulted in successful "make all" locally as well as CI pipeline.

@@ -54,8 +54,6 @@ jobs:
run: make docker-dash-p4c
- name: Build P4 software switch (bmv2) and P4Info
run: DOCKER_FLAGS=$docker_fg_flags make p4
- name: Install DASH submodules recursively
Copy link
Owner

Choose a reason for hiding this comment

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

I've made the same change in two other CI files. I believe that should be reverted as well.

Copy link
Author

Choose a reason for hiding this comment

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

Please back the changes out and we'll verify one more time with a clean local build and in CI (which is similar to a clean local build).

@@ -54,8 +54,6 @@ jobs:
run: make docker-dash-p4c
- name: Build P4 software switch (bmv2) and P4Info
run: DOCKER_FLAGS=$docker_fg_flags make p4
- name: Install DASH submodules recursively
run: git submodule update --init --recursive
Copy link
Owner

Choose a reason for hiding this comment

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

This was git submodule update --init before my changes with the comment SAI submodule update. Not sure whether it's still needed..

Copy link
Author

Choose a reason for hiding this comment

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

I think it isn't needed anymore.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants