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

Fix 'meson introspect --target-files' for a custom target #3061

Merged
merged 3 commits into from
Nov 28, 2018

Conversation

jon-turney
Copy link
Member

@jon-turney jon-turney commented Feb 13, 2018

Add testcase for, and fix meson introspect --target-files for a custom target, broken since 0.26.0 (PR #225)
For a CustomTarget, sources is a list of filenames as strings, not File objects.

Closes #2783

@jpakkane
Copy link
Member

Thanks, but this is a leftover from the bad old days when everything was a string. The "correct" solution would be to convert custom target sources to Files with self.source_strings_to_files. Can you test if this patch works for you:

diff --git a/mesonbuild/interpreter.py b/mesonbuild/interpreter.py
index 31d7616c..820090fb 100644
--- a/mesonbuild/interpreter.py
+++ b/mesonbuild/interpreter.py
@@ -2448,6 +2448,8 @@ root and issuing %s.
         if len(args) != 1:
             raise InterpreterException('custom_target: Only one positional argument is allowed, and it must be a string name')
         name = args[0]
+        if 'input' in kwargs:
+            kwargs['input'] = self.source_strings_to_files(extract_as_list(kwargs, 'input'))
         tg = CustomTargetHolder(build.CustomTarget(name, self.subdir, self.subproject, kwargs), self)
         self.add_target(name, tg.held_object)
         return tg
@@ -3009,7 +3011,7 @@ different subdirectory.
             sources = [sources]
         for s in sources:
             if isinstance(s, (mesonlib.File, GeneratedListHolder,
-                              CustomTargetHolder, CustomTargetIndexHolder)):
+                              TargetHolder, CustomTargetIndexHolder)):
                 pass
             elif isinstance(s, str):
                 self.validate_within_subproject(self.subdir, s)
diff --git a/test cases/vala/9 gir/meson.build b/test cases/vala/9 gir/meson.build
index 1a09bec2..86de2c82 100644
--- a/test cases/vala/9 gir/meson.build 
+++ b/test cases/vala/9 gir/meson.build 
@@ -11,8 +11,8 @@ foo = shared_library('foo', 'foo.vala',
                      dependencies: [glib, gobject])
 
 custom_target('foo-typelib',
-              command: [g_ir_compiler, '--output', '@OUTPUT@', '@INPUT@'],
-              input: meson.current_build_dir() + '/Foo-1.0.gir',
+              command: [g_ir_compiler, '--output', '@OUTPUT@', meson.current_build_dir() + '/Foo-1.0.gir'],
+              input: [],
               output: 'Foo-1.0.typelib',
               depends: foo)

@jpakkane jpakkane added this to the 0.44.1 milestone Feb 13, 2018
Copy link
Member

@dcbaker dcbaker left a comment

Choose a reason for hiding this comment

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

I have a few nits, but otherwise the test seems correct to me.

run_unittests.py Outdated Show resolved Hide resolved
run_unittests.py Outdated Show resolved Hide resolved
@jon-turney
Copy link
Member Author

jon-turney commented Feb 13, 2018

The "correct" solution would be to convert custom target sources to Files with self.source_strings_to_files. Can you test if this patch works for you:

Yes, that also works for custom targets in mesa.

I'm not sure about breaking vala/9 gir like that, though. Generating a typelib like that is documented, so that's just trading one regression for another...

@jpakkane
Copy link
Member

This is my bad, I have been totally asleep at the wheel to accept that. Manually building paths to files is bad. We should never promote that in our tests or official docs. I really want to fix this properly, though, because this kind of string inconsistency is a big maintenance burden.

@jon-turney jon-turney changed the title Fix 'meson introspect --target-files' for a custom target Add test case for 'meson introspect --target-files' Feb 15, 2018
@jon-turney
Copy link
Member Author

jon-turney commented Feb 15, 2018

This is my bad, I have been totally asleep at the wheel to accept that. Manually building paths to files is bad. We should never promote that in our tests or official docs. I really want to fix this properly, though, because this kind of string inconsistency is a big maintenance burden.

Ok. This PR just contains the test case now (which currently fails in CI)

@nirbheek
Copy link
Member

This is my bad, I have been totally asleep at the wheel to accept that. Manually building paths to files is bad.

Funnily enough, somehow I missed that too, so I share the blame.

@nirbheek nirbheek removed this from the 0.44.1 milestone Feb 20, 2018
@nirbheek nirbheek modified the milestones: meson-next, 0.48.0 Jul 7, 2018
jon-turney and others added 3 commits September 15, 2018 15:39
v2:
Use asssertCountEqual for list comparison ignoring order
For backwards compatibility, tolerate but warn about custom target sources
which can't be converted to Files
@jon-turney
Copy link
Member Author

jon-turney commented Sep 15, 2018

Rebased to fix conflicts.

I added something to handle non-file inputs in a backwards compatble way, with a warning:

jon@tambora /meson/test cases/vala/9 gir
$ rm -r _build ; /meson/meson.py _build
[...]
WARNING: Custom target input '/meson/test cases/vala/9 gir/_build/Foo-1.0.gir' can't be converted to a File object.
This will become a hard error in the future.

This warning should probably say something more about what's probably wrong and how to remedy things.

Generating a typelib like that is documented

I guess a defect needs to be created about how vala libraries don't expose the .gir as an output, or something like that.

@nirbheek nirbheek modified the milestones: 0.48.0, 0.49.0 Sep 20, 2018
@jon-turney jon-turney changed the title Add test case for 'meson introspect --target-files' Fix 'meson introspect --target-files' for a custom target Oct 2, 2018
@jpakkane
Copy link
Member

jpakkane commented Nov 6, 2018

I created the bug as #4481.

@jon-turney
Copy link
Member Author

I created the bug as #4481.

Thanks. Does this PR need anything further?

@jpakkane jpakkane merged commit b0611bd into mesonbuild:master Nov 28, 2018
@jon-turney jon-turney deleted the introspect-custom-target-files branch December 12, 2018 14:46
eli-schwartz added a commit to eli-schwartz/meson that referenced this pull request Nov 30, 2021
Currently there is a try/except around the function that detects and
rejects this, which instead of rejecting it, spawns a warning and
continus.

This warning exists because of 'test cases/vala/9 gir/' which passes a
vala generated output that isn't a return value (!!!) using string
joining with the meson.current_build_dir() function (also !!!) because
we officially document this (!!! for a third time) as the only way to
make a vala shared library generate a typelib with a custom_command from
the automatically generated gir:
https://mesonbuild.com/Vala.html#gobject-introspection-and-language-bindings

In mesonbuild#3061 we converted strings to Files, but only if none of them were
this vala hack. Due to the precise implementation, we also failed to
convert strings to Files if any other error occurred, but since we only
want to ignore errors for generated vala outputs, tighten that check and
specifically call out generated files in the warning.

Fixes mesonbuild#8635
eli-schwartz added a commit to eli-schwartz/meson that referenced this pull request Nov 30, 2021
Currently there is a try/except around the function that detects and
rejects this, which instead of rejecting it, spawns a warning and
continue.

This warning exists because of 'test cases/vala/9 gir/' which passes a
vala generated output that isn't a return value (!!!) using string
joining with the meson.current_build_dir() function (also !!!) because
we officially document this (!!! for a third time) as the only way to
make a vala shared library generate a typelib with a custom_command from
the automatically generated gir:
https://mesonbuild.com/Vala.html#gobject-introspection-and-language-bindings

In mesonbuild#3061 we converted strings to Files, but only if none of them were
this vala hack. Due to the precise implementation, we also failed to
convert strings to Files if any other error occurred, but since we only
want to ignore errors for generated vala outputs, tighten that check and
specifically call out generated files in the warning.

Fixes mesonbuild#8635
eli-schwartz added a commit to eli-schwartz/meson that referenced this pull request Nov 30, 2021
Currently there is a try/except around the function that detects and
rejects this, which instead of rejecting it, spawns a warning and
continue.

This warning exists because of 'test cases/vala/9 gir/' which passes a
vala generated output that isn't a return value (!!!) using string
joining with the meson.current_build_dir() function (also !!!) because
we officially document this (!!! for a third time) as the only way to
make a vala shared library generate a typelib with a custom_command from
the automatically generated gir:
https://mesonbuild.com/Vala.html#gobject-introspection-and-language-bindings

In mesonbuild#3061 we converted strings to Files, but only if none of them were
this vala hack. Due to the precise implementation, we also failed to
convert strings to Files if any other error occurred, but since we only
want to ignore errors for generated vala outputs, tighten that check and
specifically call out generated files in the warning.

Fixes mesonbuild#8635
eli-schwartz added a commit to eli-schwartz/meson that referenced this pull request Dec 7, 2021
Currently there is a try/except around the function that detects and
rejects this, which instead of rejecting it, spawns a warning and
continue.

This warning exists because of 'test cases/vala/9 gir/' which passes a
vala generated output that isn't a return value (!!!) using string
joining with the meson.current_build_dir() function (also !!!) because
we officially document this (!!! for a third time) as the only way to
make a vala shared library generate a typelib with a custom_command from
the automatically generated gir:
https://mesonbuild.com/Vala.html#gobject-introspection-and-language-bindings

In mesonbuild#3061 we converted strings to Files, but only if none of them were
this vala hack. Due to the precise implementation, we also failed to
convert strings to Files if any other error occurred, but since we only
want to ignore errors for generated vala outputs, tighten that check and
specifically call out generated files in the warning.

Fixes mesonbuild#8635
eli-schwartz added a commit to eli-schwartz/meson that referenced this pull request Dec 8, 2021
Currently there is a try/except around the function that detects and
rejects this, which instead of rejecting it, spawns a warning and
continue.

This warning exists because of 'test cases/vala/9 gir/' which passes a
vala generated output that isn't a return value (!!!) using string
joining with the meson.current_build_dir() function (also !!!) because
we officially document this (!!! for a third time) as the only way to
make a vala shared library generate a typelib with a custom_command from
the automatically generated gir:
https://mesonbuild.com/Vala.html#gobject-introspection-and-language-bindings

In mesonbuild#3061 we converted strings to Files, but only if none of them were
this vala hack. Due to the precise implementation, we also failed to
convert strings to Files if any other error occurred, but since we only
want to ignore errors for generated vala outputs, tighten that check and
specifically call out generated files in the warning.

Fixes mesonbuild#8635
Dudemanguy pushed a commit to Dudemanguy/meson that referenced this pull request Jan 21, 2022
Currently there is a try/except around the function that detects and
rejects this, which instead of rejecting it, spawns a warning and
continue.

This warning exists because of 'test cases/vala/9 gir/' which passes a
vala generated output that isn't a return value (!!!) using string
joining with the meson.current_build_dir() function (also !!!) because
we officially document this (!!! for a third time) as the only way to
make a vala shared library generate a typelib with a custom_command from
the automatically generated gir:
https://mesonbuild.com/Vala.html#gobject-introspection-and-language-bindings

In mesonbuild#3061 we converted strings to Files, but only if none of them were
this vala hack. Due to the precise implementation, we also failed to
convert strings to Files if any other error occurred, but since we only
want to ignore errors for generated vala outputs, tighten that check and
specifically call out generated files in the warning.

Fixes mesonbuild#8635
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.

mintro: AttributeError: 'str' object has no attribute 'subdir'
4 participants