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

source.py: Check for the availability of alias targets in preflight() #1862

Merged
merged 2 commits into from
Aug 31, 2023

Conversation

gtristan
Copy link
Contributor

Fixes #1860

@gtristan
Copy link
Contributor Author

Looks like there are issues with calling Source.get_source_fetchers() in preflight... might be better off raising a lazy error here rather than a preflight error...

@gtristan gtristan force-pushed the tristan/fix-fetch-mirrors-only branch from 119ef4e to 744dcf1 Compare August 21, 2023 05:47
@gtristan
Copy link
Contributor Author

gtristan commented Aug 21, 2023

Looks like there are issues with calling Source.get_source_fetchers() in preflight... might be better off raising a lazy error here rather than a preflight error...

Tried a lazy failing approach, that works but still wasn't satisfied with this.

Came back and did the check now in Source.mark_download_url(), verifying the correctness of this in the Source API contract.

Added a comment to SourceFetcher.mark_download_url() to reinforce the already existing contract, which already specified
that "This must be called for every URL in the configuration during Plugin.configure() if Source.translate_url() is not called."

This existing contract already validates the correctness of this approach to policing the alias, however it helps to reinforce the statement in the SourceFetcher docs and avoid misguiding plugin authors into thinking that SourceFetcher.mark_download_url() is a replacement of calling Source.mark_download_url()

Caveat: With this approach (including the previous approach), Buildstream will fail at load time in the case that user configuration specifies, for example, invalid tracking configuration (where aliases don't have specified URIs for tracking) when bst build or bst source fetch is called (i.e. it didn't need to be an error for the purpose of which bst was launched).

I think this is generally okay because you have to go out of your way to specify invalid tracking information anyway, so you shouldn't have invalid tracking configuration.

The current error in the test for this patch revision is:

fetch_source source at test.bst [line 3 column 2]: No fetch URI found for alias 'foo'

    Check fetch controls in your user configuration

@gtristan gtristan force-pushed the tristan/fix-fetch-mirrors-only branch from 744dcf1 to 41ae984 Compare August 21, 2023 05:53
src/buildstream/source.py Show resolved Hide resolved
This is implemented in early initialization by additionally policing
the URIs which a Source must mark with Source.mark_download_url().

Fixes #1860
@gtristan gtristan force-pushed the tristan/fix-fetch-mirrors-only branch from 41ae984 to 9434850 Compare August 23, 2023 03:12
@gtristan gtristan merged commit 97c83df into master Aug 31, 2023
16 checks passed
@gtristan gtristan deleted the tristan/fix-fetch-mirrors-only branch August 31, 2023 13:46
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.

Buildstream crashes when instructed to only download from mirrors
2 participants