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

Check the url in get_sdk_binhost before echoing #979

Merged
merged 1 commit into from
Jul 20, 2023

Conversation

krishjainx
Copy link
Contributor

@krishjainx krishjainx commented Jul 6, 2023

Check the url in get_sdk_binhost before echoing

Hi. When running the build_packages script, one can encounter an error such as 'Error fetching binhost package info from.' This pertains to SDK packages (not board packages). Since we have transitioned to the SDK container, the SDK packages are no longer published independently from the container image.

@jepio encouraged me to submit a pull request (PR) for the same

@krishjainx krishjainx changed the title Check the url in get_sdk_binhost before echoing Check the url in get_sdk_binhost before echoing [WIP] Jul 6, 2023
@krishjainx krishjainx changed the title Check the url in get_sdk_binhost before echoing [WIP] Check the url in get_sdk_binhost before echoing Jul 6, 2023
@krishjainx krishjainx temporarily deployed to development July 6, 2023 11:35 — with GitHub Actions Inactive
@jepio
Copy link
Member

jepio commented Jul 6, 2023

Can you squash the commits?

@github-actions
Copy link

github-actions bot commented Jul 6, 2023

@jepio
Copy link
Member

jepio commented Jul 7, 2023

I've tried this PR and now the following warning is printed onto the console:

!!! /etc/portage/binrepos.conf is missing (or PORTAGE_BINHOST is unset), but use is requested.
!!! /etc/portage/binrepos.conf is missing (or PORTAGE_BINHOST is unset), but use is requested.

it only makes sense to merge this if it gets rid of this warning as well.

@krishjainx
Copy link
Contributor Author

@jepio Hello, Jeremi. It appears that this error has also been resolved through my recent change. Please review it so that we can proceed with the merge. Thank you!

@jepio
Copy link
Member

jepio commented Jul 17, 2023

@jepio Hello, Jeremi. It appears that this error has also been resolved through my recent change. Please review it so that we can proceed with the merge. Thank you!

This is not a fix that we can merge. We do rely on binpkgs (and these flags) during the sdk build, but not once the sdk is pushed to the container registry. So we can't take this shortcut and remove all these flags.

You could deal with binhost being empty (or only whitespace) and then perform the necessary actions (not sure what exactly is needed to silence the warning): skip passing --getbinpkg.

Sidenote: we don't merge main into PR branches, we rebase them.

@krishjainx krishjainx temporarily deployed to development July 17, 2023 07:07 — with GitHub Actions Inactive
@krishjainx krishjainx temporarily deployed to development July 17, 2023 09:13 — with GitHub Actions Inactive
@jepio jepio closed this Jul 17, 2023
@jepio jepio reopened this Jul 17, 2023
@jepio jepio temporarily deployed to development July 17, 2023 09:20 — with GitHub Actions Inactive
When running the build_packages script, one can encounter an error such as
'Error fetching binhost package info from.' This pertains to SDK packages (not
board packages). Since we have transitioned to the SDK container, the SDK
packages are no longer published independently from the container image.
@krishjainx krishjainx temporarily deployed to development July 17, 2023 09:35 — with GitHub Actions Inactive
@jepio jepio closed this Jul 18, 2023
@jepio jepio reopened this Jul 18, 2023
@jepio jepio temporarily deployed to development July 18, 2023 08:07 — with GitHub Actions Inactive
@jepio
Copy link
Member

jepio commented Jul 20, 2023

There seem to be more similar messages but this is good enough to merge now.

@jepio jepio merged commit 75904af into flatcar:main Jul 20, 2023
6 of 10 checks passed
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