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

Add sdk-version #481

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add sdk-version #481

wants to merge 1 commit into from

Conversation

JakobDev
Copy link
Contributor

If you want to specify another SDK version than the runtime (what is needed e.g. for extensions) you currently have to use sdk: name//version. We already have things like runtime-version and base-version, so why not add sdk-version?

The code is (of course) tested and works. I hope I didn't miss anything.

@Erick555
Copy link

Erick555 commented Jul 20, 2022

Which extensions needs different Sdk version than Platform? In such case there is no abi compatibility guarantee which may lead to (un)expected brokenness. Allowing for messing this up is potential footgun.

@JakobDev
Copy link
Contributor Author

Extensions use the App the Extension is for as Runtime and a normal SDK. Because the SDK has not the same Version as the App, you need to specify the version. See this OBS plugin for example.

@Erick555
Copy link

Ok. The problem I see now is it's backwards incompatible change - all older f-b would fail to properly build new manifest. Maybe it's not worth it if there is easy workaround.

@JakobDev
Copy link
Contributor Author

I don't see why that's such a problem that old builder versions cn't build a manifest with the new key. With this argumentation we could also say that we should stop defining a new C++ standard because older versions of g++ can't compile code that uses a newer standard. Introducing YAML manifests were also a mistake, because it couldn't be build by older versions of the builder at this time.

If someone uses a older version of flatpak-builder, he uses the old syntax, when someone uses a newer version of flatpak-builder, he uses the new cleaner syntax. If someone wants to switch to an older version, it just a change of 2 lines.

@Erick555
Copy link

Erick555 commented Jul 22, 2022

The point is: why bother? Why break backward compat only to fix pure cosmetic issue? I doubt adjusting manifest for this is worth anyone time.

@JakobDev
Copy link
Contributor Author

The point here is, that on every point in the manifest you can set a Flatpak ID you also have a separate Key for a version. The SDK is the only point where you need the id//version format. So missing this here could be considered as a Bug.

@Erick555
Copy link

I could agree yet not every bug is worth fixing.

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