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

0.60 fails to build GNOME components #9441

Closed
seb128 opened this issue Oct 25, 2021 · 12 comments · Fixed by #9445
Closed

0.60 fails to build GNOME components #9441

seb128 opened this issue Oct 25, 2021 · 12 comments · Fixed by #9445

Comments

@seb128
Copy link
Contributor

seb128 commented Oct 25, 2021

Using the new meson 0.60, either from upstream or Debian, GNOME components started to fail to build

Example of the eog snap build
https://launchpadlibrarian.net/565502418/buildlog_snap_ubuntu_focal_amd64_eog_BUILDING.txt.gz

Configuring org.gnome.eog.desktop.in using configuration

../src/data/meson.build:25:5: ERROR: Function does not take positional arguments.

Trying to build the current eog deb in Ubuntu with the new meson 0.60 deb from Debian unstable leads to a similar error

@albertvaka
Copy link

albertvaka commented Oct 25, 2021

The function call that causes the error is i18n.merge_file because it's called with a positional argument. You can see an example on Nautilus (which also fails): https://gitlab.gnome.org/GNOME/nautilus/-/blob/7b469dfa/data/meson.build#L23

However, according to the docs:

This posarg is optional since 0.60.0. It defaults to the basename of the first output.

So this should be allowed. The problem might have been introduced in commit 6b1a800

@eli-schwartz
Copy link
Member

This actually changed in 61f2866.

And the docs that are relevant here are https://mesonbuild.com/i18n-module.html#i18nmerge_file

This merges translations into a text file using msgfmt. See custom_tgt for normal keywords. In addition it accepts these keywords:

It doesn't accept the positional arguments of custom_target, only the keyword arguments. However, that did not used to be type-checked, so before 0.60 if you passed a positional argument to merge_file() it would be totally ignored, but not raise an error.

@albertvaka
Copy link

albertvaka commented Oct 25, 2021

Thanks for your response! 🙂 So if I understand correctly, this argument was being silently ignored on older versions of Meson and should be removed on Gnome's side?

Maybe the merge_file docs should be updated to mention it only accepts the keyword arguments from custom_target, at least for me with the current wording this isn't obvious.

@seb128
Copy link
Contributor Author

seb128 commented Oct 25, 2021

Could the error be turned into a warning at least for a transition period to let upstreams fix their build and get new releases out?

@eli-schwartz
Copy link
Member

Maybe the merge_file docs should be updated to mention it only accepts the keyword arguments from custom_target, at least for me with the current wording this isn't obvious.

We have some very nice, modern templating for the reference manual that automatically provides function signatures with typed descriptions. Sadly, it isn't wired up to the modules documentation... Even so, anywhere else that positional arguments are accepted and used, they are mentioned, and here, their intentional absence/omission is the clue.

Could the error be turned into a warning at least for a transition period to let upstreams fix their build and get new releases out?

As I mentioned in IRC, it's very hard to know exactly which things every project out there is doing that violates the documentation, and hence is directly relying on our buggy implementation. We did publish release candidates, and ask people to use them.

@xclaesse
Copy link
Member

We should turn it into a deprecation warning for 0.60.1 and keep it removed in master. Does it break many GNOME projects? I guess it's the kind of mistake that got copy pasted everywhere?

xclaesse added a commit to xclaesse/meson that referenced this issue Oct 25, 2021
They always have been ignored but it became an hard error with no
deprecation period in 0.60.0. Since it breaks some GNOME projects,
deprecate for now and keep it removed for 0.61.0.

Fixes: mesonbuild#9441
@ondrejholy
Copy link

As per https://github.com/search?q=org%3AGNOME+merge_file&type=code, it seems that Nautilus and EOG are not definitely the only ones.

This might be interesting for @inigomartinez, who ported many of the GNOME projects...

xclaesse added a commit to xclaesse/meson that referenced this issue Oct 25, 2021
They always have been ignored but it became an hard error with no
deprecation period in 0.60.0. Since it breaks some GNOME projects,
deprecate for now and keep it removed for 0.61.0.

Fixes: mesonbuild#9441
@inigomartinez
Copy link
Contributor

I noticed it when trying upstream meson but I though that it could be an unfinished wip code because meson tries to be backwards compatible. I'll be looking forward to the fix.

Thanks @ondrejholy!

@xclaesse
Copy link
Member

The fix is there: #9445. But in the meantime please fix all GNOME projects that rely on that behaviour because it will become an hard error again in 0.61.0.

@inigomartinez
Copy link
Contributor

Wouldn't be better to let this as a deprecated warning for some more versions?

@eli-schwartz
Copy link
Member

eli-schwartz commented Oct 25, 2021

There is more than enough time between now and the next feature release of meson, for projects to fix their code.

It's a simple and backwards-compatible fix, just remove a completely ignored argument and backport it to any stable maintenance branches you have.

It's not even incorrect to fail here. It was never allowed. Meson has added a lot of improved type checking in recent years, which made many things that used to be silently ignored, turn into errors. We didn't add deprecation warnings for any of it. Anyone who is passing incorrect arguments should in fact be told what they're doing isn't actually doing what they think it is doing, and their meson.build files are buggy.

That's not a matter of backwards compatibility! That is a matter of bug-level compatibility.

This deprecation warning is being added as a courtesy gesture, because it turns out this one thing is surprisingly common among related projects and we want to give you time to fix things before your projects become unbuildable.

This is NOT permission to keep on using invalid arguments for another few years and another few major versions of meson! Fixing these bugs in the meson.build files should be prioritized IMO.

@eli-schwartz eli-schwartz linked a pull request Oct 25, 2021 that will close this issue
xclaesse added a commit to xclaesse/meson that referenced this issue Oct 26, 2021
They always have been ignored but it became an hard error with no
deprecation period in 0.60.0. Since it breaks some GNOME projects,
deprecate for now and keep it removed for 0.61.0.

Fixes: mesonbuild#9441
gnomesysadmins pushed a commit to GNOME/nautilus that referenced this issue Oct 26, 2021
The positional argument was being silently ignored until meson 0.60.0 where
it fails with "ERROR: Function does not take positional arguments".

See: mesonbuild/meson#9441
xclaesse added a commit to xclaesse/meson that referenced this issue Oct 26, 2021
They always have been ignored but it became an hard error with no
deprecation period in 0.60.0. Since it breaks some GNOME projects,
deprecate for now and keep it removed for 0.61.0.

Fixes: mesonbuild#9441
xclaesse added a commit to xclaesse/meson that referenced this issue Oct 26, 2021
They always have been ignored but it became an hard error with no
deprecation period in 0.60.0. Since it breaks some GNOME projects,
deprecate for now and keep it removed for 0.61.0.

Fixes: mesonbuild#9441
seb128 added a commit to ubuntu/evince that referenced this issue Oct 27, 2021
xclaesse added a commit to xclaesse/meson that referenced this issue Oct 27, 2021
They always have been ignored but it became an hard error with no
deprecation period in 0.60.0. Since it breaks some GNOME projects,
deprecate for now and keep it removed for 0.61.0.

Fixes: mesonbuild#9441
eli-schwartz pushed a commit that referenced this issue Oct 27, 2021
They always have been ignored but it became an hard error with no
deprecation period in 0.60.0. Since it breaks some GNOME projects,
deprecate for now and keep it removed for 0.61.0.

Fixes: #9441
@eli-schwartz
Copy link
Member

eli-schwartz commented Oct 27, 2021

Downgraded to a deprecation warning in the maintenance branch, once we release 0.60.1 you will have a whole dev cycle to find and fix these issues.

gnomesysadmins pushed a commit to GNOME/gnome-music that referenced this issue Nov 18, 2021
The positional argument was being silently ignored until meson 0.60.0
where it returns a deprecation message:
"DEPRECATION: i18n.merge_file does not take any positional
arguments. This will become a hard error in the next Meson release."

See: mesonbuild/meson#9441
gnomesysadmins pushed a commit to GNOME/gnome-music that referenced this issue Nov 18, 2021
The positional argument was being silently ignored until meson 0.60.0
where it returns a deprecation message:
"DEPRECATION: i18n.merge_file does not take any positional
arguments. This will become a hard error in the next Meson release."

See: mesonbuild/meson#9441
mbakke pushed a commit to guix-mirror/guix that referenced this issue Nov 18, 2021
See <mesonbuild/meson#9441>.

* gnu/packages/gnome.scm (eog)[arguments]: Use meson-0.59.
mbakke pushed a commit to guix-mirror/guix that referenced this issue Nov 18, 2021
See <mesonbuild/meson#9441>.

* gnu/packages/gnome.scm (gnome-characters)[arguments]: Use meson-0.59.
mbakke pushed a commit to guix-mirror/guix that referenced this issue Nov 18, 2021
See <mesonbuild/meson#9441>.

* gnu/packages/gnome.scm (gnome-font-viewer)[arguments]: Use meson-0.59.
mbakke pushed a commit to guix-mirror/guix that referenced this issue Nov 18, 2021
See <mesonbuild/meson#9441>.

* gnu/packages/gnome.scm (gnome-weather)[arguments]: Use meson-0.59.
mbakke pushed a commit to guix-mirror/guix that referenced this issue Nov 18, 2021
See <mesonbuild/meson#9441>.

* gnu/packages/gnome.scm (gnome-user-share)[arguments]: Use meson-0.59.
mbakke pushed a commit to guix-mirror/guix that referenced this issue Nov 18, 2021
See <mesonbuild/meson#9441>.

* gnu/packages/gnome.scm (totem)[arguments]: Use meson-0.59.
mbakke pushed a commit to guix-mirror/guix that referenced this issue Nov 18, 2021
See <mesonbuild/meson#9441>.

* gnu/packages/gnome.scm (simple-scan)[arguments]: Use meson-0.59.
mbakke pushed a commit to guix-mirror/guix that referenced this issue Nov 18, 2021
See <mesonbuild/meson#9441>.

* gnu/packages/gnome.scm (gnome-control-center)
[arguments]: Use meson-0.59.
[inputs]: Replace libsoup with libsoup-minimal-2.
mbakke pushed a commit to guix-mirror/guix that referenced this issue Nov 18, 2021
See <mesonbuild/meson#9441>.

* gnu/packages/gnome.scm (gnome-shell)[arguments]: Use meson-0.59.
mbakke pushed a commit to guix-mirror/guix that referenced this issue Nov 18, 2021
See <mesonbuild/meson#9441>.

* gnu/packages/gnome.scm (cheese)[arguments]: Use meson-0.59.
FalkAlexander pushed a commit to FalkAlexander/PasswordSafe that referenced this issue Dec 19, 2021
The positional argument was being silently ignored until meson 0.60.0
where it returns a deprecation message:
"DEPRECATION: i18n.merge_file does not take any positional
arguments. This will become a hard error in the next Meson release."

See: mesonbuild/meson#9441
andyholmes added a commit to andyholmes/valent that referenced this issue Dec 26, 2021
* bump meson dependency to 0.56 and fix deprecations
* reorder root meson.build
* fix build on clang, passing `-Wno-error=unused-but-set-variable` to
  avoid false positives with cleanup functions
* remove unused project arguments `-DVALENT_COMPILATION` and
  `-DHAVE_CONFIG_H`
* remove application name suffix for devel builds
* fix bogus positional argument on i18n.merge_file()
  see: mesonbuild/meson#9441
* misc style cleanups
andyholmes added a commit to andyholmes/valent that referenced this issue Dec 26, 2021
* bump meson dependency to 0.56 and fix deprecations
* reorder root meson.build
* fix build on clang, passing `-Wno-error=unused-but-set-variable` to
  avoid false positives with cleanup functions
* remove unused project arguments `-DVALENT_COMPILATION` and
  `-DHAVE_CONFIG_H`
* remove application name suffix for devel builds
* fix bogus positional argument on i18n.merge_file()
  see: mesonbuild/meson#9441
* misc style cleanups
andyholmes added a commit to andyholmes/valent that referenced this issue Dec 27, 2021
* bump meson dependency to 0.56 and fix deprecations
* fix build on clang, passing `-Wno-error=unused-but-set-variable` to
  avoid false positives with cleanup functions
* remove unused project arguments `-DVALENT_COMPILATION` and
  `-DHAVE_CONFIG_H`
* remove application name suffix for devel builds
* fix bogus positional argument on i18n.merge_file()
  see: mesonbuild/meson#9441
* misc style cleanups
andyholmes added a commit to andyholmes/valent that referenced this issue Dec 27, 2021
* bump meson dependency to 0.56 and fix deprecations
* fix build on clang, passing `-Wno-error=unused-but-set-variable` to
  avoid false positives with cleanup functions
* remove unused project arguments `-DVALENT_COMPILATION` and
  `-DHAVE_CONFIG_H`
* remove application name suffix for devel builds
* fix bogus positional argument on i18n.merge_file()
  see: mesonbuild/meson#9441
* misc style cleanups
andyholmes added a commit to andyholmes/valent that referenced this issue Dec 27, 2021
* bump meson dependency to 0.56 and fix deprecations
* fix build on clang, passing `-Wno-error=unused-but-set-variable` to
  avoid false positives with cleanup functions
* remove unused project arguments `-DVALENT_COMPILATION` and
  `-DHAVE_CONFIG_H`
* remove application name suffix for devel builds
* fix bogus positional argument on i18n.merge_file()
  see: mesonbuild/meson#9441
* misc style cleanups
gnomesysadmins pushed a commit to GNOME/eog that referenced this issue Dec 29, 2021
Since meson 0.60, the name in `custom_target`, and derivatives, is
optional[0], although the implementation has produced some errors at
the moment[1].

Due to this, the name has been removed when possible.

When possible, two pass targets, where there is a first pass to
replace variables, and second pass to translate files, have been
modified to be avoid extra build commands.

The `@BASENAME@` token has also been used as an approach to remove
the `in` suffix.

[0] https://mesonbuild.com/Release-notes-for-0-60-0.html#optional-custom_target-name
[1] mesonbuild/meson#9441
gnomesysadmins pushed a commit to GNOME/gvfs that referenced this issue Jan 21, 2022
The positional argument was being silently ignored until meson 0.60.0 where
it fails with "ERROR: Function does not take positional arguments".

Related: mesonbuild/meson#9441
Fixes: https://gitlab.gnome.org/GNOME/gvfs/-/issues/599
ptrcnull added a commit to ptrcnull/gtkhash that referenced this issue Mar 3, 2022
As per mesonbuild/meson#9441,
these arguments were never used and were made deprecated in Meson 60.
fabiscafe pushed a commit to fabiscafe/fcgu that referenced this issue Mar 7, 2022
gnomesysadmins pushed a commit to GNOME/gvfs that referenced this issue May 26, 2022
The positional argument was being silently ignored until meson 0.60.0 where
it fails with "ERROR: Function does not take positional arguments".

Related: mesonbuild/meson#9441
Fixes: https://gitlab.gnome.org/GNOME/gvfs/-/issues/599

(cherry picked from commit 17a067b)
mmstick pushed a commit to pop-os/network-manager-applet that referenced this issue Aug 1, 2024
The positional argument was being silently ignored until meson 0.60.0 where
it fails with "ERROR: Function does not take positional arguments".

It was temporarily downgraded to a warning in 0.60.1 but became a
hard failure again in 0.61.0.

See: mesonbuild/meson#9441

Patch pulled from https://gitlab.gnome.org/GNOME/network-manager-applet/-/merge_requests/107

Closes: #1005529
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 a pull request may close this issue.

6 participants