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 win_subsystem kwarg. Closes #7765. #7791

Merged
merged 1 commit into from
Oct 7, 2020
Merged

Add win_subsystem kwarg. Closes #7765. #7791

merged 1 commit into from
Oct 7, 2020

Conversation

jpakkane
Copy link
Member

No description provided.

Comment on lines 224 to 227
if 'windows' in value:
return ['-mwindows']
elif 'console' in value:
return ['-mconsole']
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't there be a warning for something like windows,6.0, since the 6.0 part is ignored, or is this irrelevant for MinGW?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GCC documentation does not say anything about specifying a version. So I don't really know. I'm guessing people who care about this detail will only use MSVC anyway.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These things are controllable via ld flags: see --subsystem, --major-subsystem-version, --minor-subsystem-version in man ld.

If you look at the output of gcc -dumpspecs you can see that -mwindows is a convenient shortcut for --subsystem windows and some libraries.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rewriting this but currently msys download page is unresponsive so I can't actually test it locally but instead have to use CI. :(

@@ -673,6 +674,12 @@ be passed to [shared and static libraries](#library).
- `pie` *(since 0.49.0)*: build a position-independent executable
- `native`: is a boolean controlling whether the target is compiled for the
build or host machines. Defaults to false, building for the host machine.
- `win_subsystem` *(since 0.56.0)* specifies the subsystem type to use
on the Windows platform. Typical values include `console` for text
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unclear if numeric values are expected to work here, so perhaps is a string specifying the subsystem...?

@jpakkane jpakkane force-pushed the winsubsystem branch 4 times, most recently from 6b66c4c to d6cc358 Compare October 5, 2020 16:03
@jpakkane
Copy link
Member Author

jpakkane commented Oct 5, 2020

That should be all done now. @dcbaker this probably means that you need to add some sort of a gui kwarg to the freedesktop module. Is that ok?

@dcbaker
Copy link
Member

dcbaker commented Oct 6, 2020

Yeah, that's fine. I was just sorta piggy backing on it because it was convenient.

@jpakkane jpakkane merged commit 1a06038 into master Oct 7, 2020
@jpakkane jpakkane deleted the winsubsystem branch October 7, 2020 15:55
@bonzini
Copy link
Contributor

bonzini commented Oct 19, 2020

The instant deprecation is annoying, it triggers about 60 times when building QEMU. Because 0.55.x doesn't have the win_subsystem kwarg at all, it means we cannot support 0.55.x and 0.56.x at the same time without playing games with kwargs (and frankly those games are not why I choose Meson for my project).

Now, for QEMU the situation is not that bad: we distribute our own copy of Meson (which makes it much easier to stay on the bleeding edge), we haven't even had a release yet that required Meson, and we probably want to adopt 0.56.0 ASAP anyway due to #7760. But then, distros may not want to use the embedded QEMU, and in any case we want to work with upstream as much as possible to share our pain points even for the future.

I'll create a pull request to restrict the deprecation message to meson_version: '>=0.56.0' for a couple releases, but long term there should be at least three different kinds of deprecation:

  • cases where Meson is doing something that will break immediately (e.g. also in 0.56.0, replacing colons in test names)

  • cases where something willl be removed in the future because it was a bad idea

  • cases where the syntax has a simple replacement.

For the last two, it's probably time for Meson to have a formal deprecation policy. The policy should specify:

  • a minimum period between warning and removal

  • a minimum preriod between introducing the alternative and warning (unless a high-enough meson_version is specified).

@mensinda
Copy link
Member

The instant deprecation is annoying, it triggers about 60 times when building QEMU.

Would the situation improve if all deprecations were only logged once by default? I understand that the situation is suboptimal, but I see no reason not to deprecate an old function immediately if there is a new and better alternative. As for restricting the warning to meson_version: '>=0.56.0', I think this is a good idea, however, this restriction should be dropped after <insert number> of releases or <insert number> of releases before the feature is removed.

@bonzini
Copy link
Contributor

bonzini commented Oct 19, 2020

I see no reason not to deprecate an old function immediately if there is a new and better alternative

Yeah in the end I agree and @FeatureDeprecatedKwargs does what I need, so I used it in #7872. Making the warning independent of meson_version() can be done later (possibly by not passing any version to the decorator?).

this restriction should be dropped after <insert number> of releases or <insert number> of releases before the feature is removed.

That <insert number> is exactly why having a policy would be great. :)

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

5 participants