Add snippets.symbol_visibility_header() method

[xclaesse]

Defining public API in a cross platform library is painful, especially on Windows. Since every library have to define pretty much the same macros, better do it in Meson.

[xclaesse]

Cases I'm unsure: clang-cl? cygwin? msys2 gcc?

[dcbaker]

Question, why not put this in the windows module?

[xclaesse]

Question, why not put this in the windows module?

I did that initially, then saw that we have gnu visibility to deal with too. The pain is not limited to Windows.

[tristan957]

@dcbaker this also defines the GCC/clang attributes.

[codecov[bot]]

Codecov Report

Merging #10199 (4a65fda) into master (b85ffba) will decrease coverage by 16.89%. The diff coverage is n/a.

:exclamation: Current head 4a65fda differs from pull request most recent head 31d788e. Consider uploading reports for the commit 31d788e to get more accurate results

@@             Coverage Diff             @@
##           master   #10199       +/-   ##
===========================================
- Coverage   67.07%   50.18%   -16.89%     
===========================================
  Files         420      203      -217     
  Lines       91294    43779    -47515     
  Branches    20277     9710    -10567     
===========================================
- Hits        61234    21971    -39263     
+ Misses      25309    19823     -5486     
+ Partials     4751     1985     -2766     

see 425 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[xclaesse]

I changed default values to use @nirbheek 's suggestion of using the project name.

[jpakkane]

This is such a notable feature that it should get a mention in the proper docs as well, not only in the reference part.

[dcbaker]

Okay, then this should really live in the clike compiler mixin not in the base compiler

[xclaesse]

This is such a notable feature that it should get a mention in the proper docs as well, not only in the reference part.

Added a mention in gnu_symbol_visibility kwarg doc, do you have other places in mind?

Okay, then this should really live in the clike compiler mixin not in the base compiler

I wasn't sure it's worth moving implementation into Compiler class, but since it needs different code for at least msvc and gnu, that would make sense indeed.

[tristan957]

Thinking about naming consistency...I added java.generate_native_headers(). Wondering if I should have named it java.native_headers() instead similar to how this PR is cc.symbol_visibility_header(). No verb. Naming is hard :(

[xclaesse]

Added static_only kwarg, when set to true _STATIC_COMPILATION gets defined in the header, so no need to define it manually in cflags and pkgconfig file any more. That's how glib does it.

Also I fixed the doc, installing the generated header is always required. I was wrong to think it was a project choice.

[xclaesse]

No verb. Naming is hard :(

I think most of the time we indeed avoid verbs in Meson API, there are very few get_foo(). But that's not entirely consistent.

[tristan957]

Oh well, can always be fixed in a future release.

[xclaesse]

This is essentially cmake's GENERATE_EXPORT_HEADER()

I did not know cmake has this, do you have an example of project that uses that? I'm currious of what it contains.

[eli-schwartz]

https://github.com/mesonbuild/wrapdb/pull/309

[xclaesse]

mesonbuild/wrapdb#309

Lol, cmake does it even weirder, they define it twice the same way...

#    ifdef libhighs_EXPORTS
        /* We are building this library */
#      define LIBHIGHS_EXPORT __attribute__((visibility("default")))
#    else
        /* We are using this library */
#      define LIBHIGHS_EXPORT __attribute__((visibility("default")))
#    endif

[xclaesse]

Okay, then this should really live in the clike compiler mixin not in the base compiler

@dcbaker done now.

[xclaesse]

Finally passed the CI, ready for review.

[tristan957]

I like the idea of this function existing in a clike module rather than on the compiler itself.

[xclaesse]

Anyone for a review please?

[xclaesse]

Updated this PR for 1.3.0. AFAIK there is no strong opposition, just slight concern that it might not be as useful as it seems?

[eli-schwartz]

Updated this PR for 1.3.0. AFAIK there is no strong opposition, just slight concern that it might not be as useful as it seems?

The objection was more than slight concern, but it was also about the belief that this is badly suited to being a compiler attribute and better suited to being e.g. a module named something like "codesnippets".

[xclaesse]

@eli-schwartz what other method would you see in that module? I never saw a concrete proposal, and making a module for a single method seems overkill. IMHO a compiler method is by far the most intuitive place, I really don't why that's a problem.

[dnicolodi]

making a module for a single method seems overkill

Well, technically, the python module exposes only one function... 😄

[xclaesse]

Updated this PR to add snippets module.

[tristan957]

May want to change the PR title too!

[tristan957]

Here is a stupid question. Should we add a license header to the generated header? Something like Unlicense or CC0? Or add a preamble/prefix kwarg of some sort?

[ePirat]

Should we add a license header to the generated header? Something like Unlicense or CC0? Or add a preamble/prefix kwarg of some sort?

IMO the best solution to this and the #pragma vs classic include guards issue is to add the possibility to allow to just get the string so projects can configure_file their own header with their own include guards, license and whatever else they might want to add.

[eli-schwartz]

We don't add a license for generated config.h files because they're just a list of constants that have been format-shifted into C preprocessor statements. I can see the argument that this produced header is a bit more, well, creative work.

What's the purpose of the license in this case, then? Do we want to explicitly disavow any copyright claim against people using the produced header? Probably, that's the usual reason to go with a license that claims to be equivalent to a public domain dedication.

The frustrating thing is that there's a lot of disagreement about what kinds of public domain dedication "count". Various groups will insist that CC0 isn't free enough, or that unlicense isn't "clear", etc. For example, Google says its employees cannot contribute to projects under either of those two licenses as part of its internal policy guidance on:

Forbidden patches

Our general philosophy is that we do not allow patches to projects that Google cannot use, either because of the terms of the license or Google policy.

[xclaesse]

AFAIK if we want a license it should be Meson's Apache 2.0, because that's basically copying a string from Meson source into a file. I wouldn't mind adding /* SPDX-License-Identifier: Apache-2.0 */ line on top.

But then raise the question of where did I found that code snippet... it is inspired from what I saw in glib/gstreamer (LGPL2+), but not exactly (because I had to fix them in the process). It is also inspired from GCC documentation https://gcc.gnu.org/wiki/Visibility (GPL?).

[ePirat]

AFAIK if we want a license it should be Meson's Apache 2.0, because that's basically copying a string from Meson source into a file. I wouldn't mind adding /* SPDX-License-Identifier: Apache-2.0 */ line on top.

This will be unusable at least in VLC then, as it would force the rest of the code to (L)GPL 3 to be compatible and I am not even sure if that compatibility extends to source-level and not just linking…

[xclaesse]

Let's just do nothing and everyone will be happy. As the author of that snippet I hereby claim that everyone can do what the fuck they want with it: SPDX-License-Identifier: WTFPL :D

[xclaesse]

More seriously, I think it's fine for GPL3: https://en.wikipedia.org/wiki/Apache_License#Compatibility

[eli-schwartz]

Let's just do nothing and everyone will be happy. As the author of that snippet I hereby claim that everyone can do what the fuck they want with it: SPDX-License-Identifier: WTFPL :D

That too is on the Google "do not use" list. :P My understanding is that Google is mad that it isn't actually Public Domain, so the code is still provided under a "license", but the license says you can "do whatever you want" but doesn't say you can "have a warranty disclaimer". :facepalm:

Their documentation on it appears to have vanished but the Internet Archive does not forget:

Code released under the WTFPL cannot be used at Google. This license has a large number of issues (lack of warranty disclaimer, very vague rights grant), and was also rejected as an open source license by OSI. We also do not allow contribution to projects under the WTFPL.

Perhaps its disappearance is a sign that it became acceptable to use, but I doubt it since it is still listed as one of the licenses that Googlers aren't allowed to contribute to: https://opensource.google/documentation/reference/patching#forbidden

BSD0 is however on the approved list as an explicit exception to "we at Google do not like Public Domain dedications", and is commonly considered to be "not actually a Public Domain dedication but effectively equivalent".

[eli-schwartz]

Let's just do nothing and everyone will be happy.

And in particular this: I am not overly worried about whether or not people can freely use a function to generate totally optional code snippets, but "do nothing and everyone will be happy" is an incredibly humorous thought, because this is a topic that people make a very good livelihood off of convincing the entire tech world to be deeply unhappy about it.

[xclaesse]

Anyway, let me know what you want, I couldn't care less. I'm not a lawyer.

[tristan957]

IMO the best solution to this and the #pragma vs classic include guards issue is to add the possibility to allow to just get the string so projects can configure_file their own header with their own include guards, license and whatever else they might want to add.

I really like this idea. @xclaesse what if we made this more similar to vcs_tag()?

[xclaesse]

Returning the string to be able to add it into an existing configure_file() would make sense, that's the kind of things many project already have in their config.h. IMHO that would require to add at least fs.write() method to be able to write that string verbatim into a new file, and also have proper solution for https://github.com/mesonbuild/meson/issues/6259.

I don't think that solves the license question, the problem is exactly the same, are you allowed to paste that code into a file with your project license. But unless one of us is secretly a lawyer specialized on that matter, I think we are just talking without knowing. If a big company wants to pay a few millions their legal team to debate on this we'll probably have a answer in a decade or two.

[tristan957]

What I am thinking is more along the lines of:

// header.h.in

// preable...

@SYMBOL_VISIBILITY_MACROS@

// function decls, etc.

header = snippets.symbole_visibility_macros(input: 'header.h.in', output: 'header.h')

[xclaesse]

That makes API more complicated to use for no real benefit IMHO. Hooking it with configure_file() does have the benefit to make the snippet part of an already existing file in the project instead of adding an extra one.

[tristan957]

The benefit I see is that in the current approach all projects now have to install a visibility.h into the includedir. While the proposed approach allows them to embed in any file they want.

[xclaesse]

Sure, but then doing it through configure_file() seems much more flexible because you can substitute other keys in a single step.

[tristan957]

Sure, but then doing it through configure_file() seems much more flexible because you can substitute other keys in a single step.

Right. This is similar the vcs_tag() idea I talked about on Matrix