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

Enable binary compatibility validation for KLibs #929

Merged
merged 3 commits into from
Apr 12, 2024
Merged

Conversation

@DRSchlaubi
Copy link
Member

I looked at this yesterday (before I sent you the news ^-^) however, there is an issue regarding #855, because K/N can only produce API dumps for targets it can compile for, this isn't a big deal for the check task, as we run on all hosts anyways, however now every PR needs to produce a dump for Linux, Windows and MAC, one way to fix this, would be to add a GitHub bot (via GitHub Actions or TeamCity) that will do that for you, if you comment something like /dump-api, that will produce the dump for you, however there is still an issue regarding that approach, as it looks like the dump is a single file, so 3 different CI runs would need to produce a single combined file

@lukellmann
Copy link
Member Author

i haven't experimented with native, but won't it just work for common code?

@DRSchlaubi
Copy link
Member

No as it dumps the platform API not the common api

@lukellmann
Copy link
Member Author

running this on the feature/native branch on my windows machine, i get this:

> Task :common:macosArm64ApiInfer
An ABI dump for target macosArm64 was inferred from the ABI generated for the following targets as the former target is not supported by the host compiler: [linuxArm64,linuxX64,mingwX64]. Inferred dump may not reflect an actual ABI for the target macosArm64. It is recommended to regenerate the dump on the host supporting all required compilation target.

so we only need an apple machine if we would want to add apple only api

@DRSchlaubi
Copy link
Member

When I did it it failed

@lukellmann
Copy link
Member Author

because the linux targets are disabled, there is no target left for voice and core-voice except apple. using wsl or enabling the targets on windows for dumping fixes this.

@DRSchlaubi
Copy link
Member

because the linux targets are disabled, there is no target left for voice and core-voice except apple. using wsl or enabling the targets on windows for dumping fixes this.

Neither of which we can do, but that does indeed explain it

@lukellmann
Copy link
Member Author

Neither of which we can do, but that does indeed explain it

it's at least a workaround so we don't have to build a bot for this. at some point the issue with linux targets may be fixed (?) or we will add js/windows targets to voice. then the dump can be inferred from there.

@DRSchlaubi
Copy link
Member

The issue is with curl on windows, which will most definitely never get fixed

The workaround for :core:live-tests is no longer needed.
DRSchlaubi added a commit that referenced this pull request Apr 12, 2024
DRSchlaubi added a commit that referenced this pull request Apr 12, 2024
@DRSchlaubi DRSchlaubi merged commit d9ad58e into main Apr 12, 2024
8 of 9 checks passed
@lukellmann lukellmann deleted the klib-abi-dump branch April 12, 2024 23:30
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.

None yet

2 participants