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

Differentiate reasons for build failures, specifically adding a "Swift tools version" reason #3383

Open
daveverwer opened this issue Sep 11, 2024 Discussed in #3381 · 4 comments
Labels
enhancement New feature or request

Comments

@daveverwer
Copy link
Member

First discussed in #3381

Then, skipping builds based on tools version. The interesting thing about #2294, #2125, and #727 is that we didn't consider adding a different status for those builds that we skip, and so there was no benefit other than to save us some CPU time in our build infrastructure. What might make the difference is if we consider a "tools version failure" a different kind of "failure" to a regular build error. We already extract and store tools version in our package version metadata, so it wouldn't be too hard to insert a tools version failure record before triggering a build.

We could also limit the impact of the changes by making the new "tools version failure" status as associated value enumeration inside our failure state rather than a completely new state, creating "reason for failure" metadata. We could still treat as a failure everywhere we currently do, but that could be displayed differently on the build detail page.

The more I think about this, the more I think we should do it. Scoping this to being a failure reason rather than a full new state for a build to be in should make the impact of the change much simpler and it would be a step forward for our build reporting, as well as giving us the minor speed increase we'd get from not round-tripping to the build infrastructure.

@daveverwer daveverwer added the enhancement New feature or request label Sep 11, 2024
@finestructure
Copy link
Member

I think adding a build with status unsupported is a great idea. The reason I abandoned not triggering builds that have outdated tools versions was that the trigger query would get way too complicated if we did that.

However, we can just insert a build record during analysis when we first discover a new version. That way we don't need to touch the trigger query at all. It'll just not select those builds to begin with.

In terms of how to record it, I think we should aim for giving it a proper status like unsupported. Given it's an enum we'll see where it's being used and where we might face trouble.

But by giving it a new case we'll have an easier time extending the result representation in the build details page where we can then easily show those builds as unsupported instead of failed.

This would also allow us to perhaps extend this to platforms in the future. We've had a couple of inquiries how to mark a platform as unsupported and so far the recommendation was to add an #if ... #error .... With this mechanism in place, we could add a new key to .spi.yml where authors can declare a platform unsupported and we skip all its builds in exactly the same way as we would for tools-version.

@daveverwer
Copy link
Member Author

In terms of how to record it, I think we should aim for giving it a proper status like unsupported. Given it's an enum we'll see where it's being used and where we might face trouble.

My thought was that this might make our aggregation up to the matrix more complex, but it works either way.

@adincebic
Copy link

@daveverwer I should have some free time starting Sunday and would like to tackle this.

@daveverwer
Copy link
Member Author

@daveverwer I should have some free time starting Sunday and would like to tackle this.

Thank you for the offer of help! We are always happy to accept contributions. This one might be a little tricky to do as a first issue, but let's chat about it. If you use Discord, join us on our server and we can chat about it.

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

No branches or pull requests

3 participants