-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Platform/cisco-8000 module for sonic-buildimage #8172
Conversation
…lude file Signed-off-by: Venkat Garigipati <venkatg@cisco.com>
@VenkatCisco , is the PR similar to #7821 ? |
PLATFORM_PATH := platform/$(if $(PLATFORM),$(PLATFORM),$(CONFIGURED_PLATFORM)) | ||
PLATFORM_CHECKOUT := platform/checkout | ||
PLATFORM_CHECKOUT_FILE := $(PLATFORM_CHECKOUT)/$(PLATFORM).ini | ||
PLATFORM_CHECKOUT_CMD := $(shell if [ -f $(PLATFORM_CHECKOUT_FILE) ]; then PLATFORM_PATH=$(PLATFORM_PATH) j2 $(PLATFORM_CHECKOUT)/template.j2 $(PLATFORM_CHECKOUT_FILE); fi) |
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.
To support azp, please allow the step can be skipped when the repo has already checked out.
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.
To support azp, please allow the step can be skipped when the repo has already checked out.
@xumia would the change in platform/checkout/template.j2
be sufficient to support azp?
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.
This step will automatically be skipped if the $(PLATFORM_PATH)
directory already exists in the workspace
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.
Request change to Makefile
.
@VenkatCisco , I created a pipeline for test only, please refer to the azp definition for the code checkout logic, thanks. The error is as below:
The PR looks good now, it may be only a submodule repo commit issue. |
Signed-off-by: Venkat Garigipati <venkatg@cisco.com>
@xumia, the PR is updated based on the use case you mentioned. It also accommodates adjustments to the Makefile based on the new bullseye addition. Please review. |
How to build 'sonic-cisco-8000.bin' -make init Ensure the platform/checkout/cisco-8000.ini has the right repo & tag versions that is been shared |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
@VenkatCisco , could you please run a test to build sonic-cisco-8000.bin? I tested in my local dev env, it was failed with the same result as I mentioned above.
Looks like the PR missing a step to download artifactory-v0.1.tar.gz as: https://github.com/Cisco-8000-sonic/platform-cisco-8000/releases |
@xumia The build instruction is included in the private Cisco-8000-sonic github organization. It is not part of this PR (in which the intent is to integrate a private platform-specific repository into azure/sonic-buildimage during Please review and merge if it is LGTM (and we also request a double-commit to 202012 branch). Thank you. |
@huanlev , it works fine on master branch, but not good in 202012 branch, could you please take a look? |
@huanlev , for the release package, we need to add a step in azp to download it (see above links), is there any better solution, similar to download the other web packages during the build? |
@qiluo-msft , could you please help review it? |
@xumia I don't have access to the 202012 (Cisco-8000-sonic) URL. Can you help giving me read permission? |
@xumia @qiluo-msft thanks for reviewing and approving the PR. |
I think we can merge it, it should have no impact on the other platforms and the 202012 branch issue can be fixed later (not quite relative to this PR). For the azp to download the GitHub release artifacts, it is not very good, we need to do additional steps for cisco-8000, but it is acceptable. |
Why I did it Update Makefile, so it does the following: For a given platform, verify if platform/checkout/.ini exists and hence run the platform/checkout/template.j2. This allows platform code to be checked out during the 'make configure' stage. How I did it git clone git@github.com:Azure/sonic-buildimage.git mkdir platform/cisco-8000 make init make configure PLATFORM=cisco-8000 make all
This PR could not be cleanly cherry-pick to 202012. Please submit another PR. |
Why I did it Update Makefile, so it does the following: For a given platform, verify if platform/checkout/.ini exists and hence run the platform/checkout/template.j2. This allows platform code to be checked out during the 'make configure' stage. How I did it git clone git@github.com:Azure/sonic-buildimage.git mkdir platform/cisco-8000 make init make configure PLATFORM=cisco-8000 make all
We will submit another PR for 202012. Thank you Qi! |
Hi Qi, submitted PR #8399 |
Why I did it
Update Makefile, so it does the following:
For a given platform, verify if platform/checkout/.ini exists and hence run the platform/checkout/template.j2. This allows platform code to be checked out during the 'make configure' stage.
How I did it
git clone git@github.com:Azure/sonic-buildimage.git
mkdir platform/cisco-8000
make init
make configure PLATFORM=cisco-8000
make all
How to verify it
Ensure, the platform/checkout/.ini file refers to the correct github page with the correct version tag that can be downloaded.
Which release branch to backport (provide reason below if selected)
Description for the changelog
A picture of a cute animal (not mandatory but encouraged)