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

Qt compile_moc() doesn't support files with the same name #10955

Open
punytroll opened this issue Oct 25, 2022 · 21 comments
Open

Qt compile_moc() doesn't support files with the same name #10955

punytroll opened this issue Oct 25, 2022 · 21 comments

Comments

@punytroll
Copy link
Contributor

punytroll commented Oct 25, 2022

Describe the bug

When using Qt's compile_moc(), files that have the same file name, while being placed in different sub directories in the source tree, produce a configure-time error:

ERROR: Multiple producers for Ninja target "app.p/moc_value.cpp". Please rename your targets.

or

ERROR: Multiple producers for Ninja target "app.p/value.moc". Please rename your targets.

depending on whether one is using the headers or sources keyword argument.

To Reproduce

This minimal meson.build file:

project('bug', 'cpp')
qt = import('qt6')
app_sources = files('app.cpp')
app_sources += qt.compile_moc(sources: [
    'one/value.h',
    'two/value.h'])
executable('app', sources: app_sources)

Files one/value.h and two/value.h must exist, but can, for this error at configure time, be empty.

Expected behavior

There is no technical reason, why this shouldn't work. CMake can do it and a handcrafted Makefile certainly can as well. The moc files need either be placed in separate directories (CMake creates unique build directories for this) and the include paths to these directories have to be provided at compile time. Or the filenames have to be prefixed with the directory ('one_value.moc' / 'two_value.moc'), which seems more mesonic, but creates a problem when you want to include these files again ('#include "one_value.moc"' instead of the canonic '#include "value.moc"' in both files, trusting the include paths to be correct.)

system parameters

  • Native linux build.
  • python 3.10.4
  • meson 0.63.3
  • ninja 1.10.1
@punytroll
Copy link
Contributor Author

Updated to meson 0.63.3. Problem remains.

@tristan957
Copy link
Contributor

tristan957 commented Oct 26, 2022

Given a non-qt project:

srcdir
├── include
│   ├── hse
│   │   └── pidfile
│   │       └── pidfile.h
│   └── meson.build
├── lib
│   ├── meson.build
│   └── pidfile.c
└── meson.build
builddir
├── include
├── lib
├── libhse-pidfile.a
└── libhse-pidfile.a.p
    └── lib_pidfile.c.o

You can see that the folder name is prefixing the pidfile.c name in the private dir. I think all the moc stuff needs to do is something similar

@tristan957
Copy link
Contributor

The bug is located right around here https://github.com/mesonbuild/meson/blob/master/mesonbuild/modules/qt.py#L463

@punytroll
Copy link
Contributor Author

punytroll commented Oct 27, 2022

Aye, that's what I thought.
Is it open for debate whether the prefix is embedded in the filename or in a directory structure?

Because, as mentioned in the issue, I recognize that something like "one_value.moc" would be more mesonic, but that is not what Qt developers would expect! Traditionally, these generated moc files would be #included as "value.moc" inside of the cpp file. However, if the filename is extended with directory prefixes that doesn't work anymore ...

So, would an implementation for this issue be free to create subdirectories inside the private directory and append those to the include directories?

I hope I'm making myself clear enough ... ;)

@tristan957
Copy link
Contributor

So you have something like this?

src/a/meson.build compiles moc.

src/b/meson.build compiles moc.

And both generate the same file name in builddir/?

@punytroll
Copy link
Contributor Author

Yes, even if I place individual meson.build files inside the subdirectories one and two it wants to create two files app.p/value.moc. I've tried that before but forgot about that.

However, I am convinced that both approaches should be handled correctly by meson:

  1. placing individual meson.build files with compile_moc() in the subdirectories
  2. placing a single meson.build file with a combined compile_moc() in any parent directory

@jpakkane
Copy link
Member

Regardless of everything else, I highly recommend adopting a naming scheme that prohibits having multiple headers of the same name. Murphy's law says that eventually you will get that wrong and include the wrong header from the wrong place. The only real protection against that is to have unique names.

@tristan957
Copy link
Contributor

@punytroll the obvious solution to me seems to be generating the files into builddir/{a,b}/ instead of builddir.

@punytroll
Copy link
Contributor Author

punytroll commented Oct 27, 2022

In general, that might be a sensible recommendation.

On the other hand, structuring your code in subdirectories accoring to your namespace names and files according to your class names is not that absurd an idea. And it's only a matter of time until some (dreaded) Manager, Parser or Value class comes along ... If the developer team gets big enough or the project gets complex enough, I don't think you can sensibly enforce this rule.

And in any case, namespaces provide good protection against accidentally including and using the wrong header without the compiler complaining. I don't think protecting against that falls into the realm of responsibilities for the build system.

@punytroll
Copy link
Contributor Author

punytroll commented Oct 27, 2022

@tristan957 I think so as well, but I wanted to make sure, that subdirectories in the build directory are an OK thing to do.

I will have a stab at it.

@tristan957
Copy link
Contributor

@punytroll generally writing output files to the mirrored build directory is the right thing to do

@eli-schwartz
Copy link
Member

Yes, even if I place individual meson.build files inside the subdirectories one and two it wants to create two files app.p/value.moc. I've tried that before but forgot about that.

app.p is a per-target private directory.

If this were handled via manual running of generator().process() with find_program('moc') as the command to run, then all files will be generated into process() has a preserve_path_from: kwarg, which allows the input one/value.h to be transformed into app.p/one/moc_value.cpp; this is fine, because although Meson doesn't generally allow outputting files into subdirectories, it allows it when operating inside the private directory for a target.

It's perfectly reasonable to argue that the builtin moc should always do that too.

@tristan957
Copy link
Contributor

Would make sense to me

@jpakkane
Copy link
Member

Or it could have the preserve kwarg so people could opt into it. Changing the default would break things for existing projects.

@eli-schwartz
Copy link
Member

eli-schwartz commented Oct 27, 2022

Since we are talking about generated .cpp files, the risk of that is probably pretty low... although there's no harm in making it optional I guess.

@jpakkane
Copy link
Member

Hmm. Wait, does moc produce only source files or headers too? If the latter then we can't change the output names, but if it is only the former then probably yes.

@punytroll
Copy link
Contributor Author

punytroll commented Oct 27, 2022

Well, that depends on how you look at that:

  1. when using compile_moc() with the headers keyword, then the output is a cpp file, internal to the meson build process, that meson automatically compiles and links into the target. The problem of this issue applies here but the actual filename is never exposed or referred to.
  2. when using compile_moc() with the sources keyword, then the output is a .moc file that needs to be included manually in the cpp source file. The moc file, though being an implementation file, is #included, so behaviorally it is a header file as well. The problem of this issue applies as well.

In the latter case, the name of the generated file should definitely not be changed from the expected file name (<basefilename>.moc), because that file name is probably referred to in another source file. The directory of the generated file can be changed of course, as long as the resulting directory is appended to the include_directories of the target.

@punytroll
Copy link
Contributor Author

I have to agree with @eli-schwartz: As this pertains to generated files only, is there any chance it could break something for existing projects? As long as the created subdirectories are appended to the target's include_directories, the same file names should be visible to the compiler to include ...

@punytroll
Copy link
Contributor Author

I had a quick glance at this:

It seems passing preserve_path_from = state.environment.source_dir to Generator.process_files() works quite nicely. But only when the headers kwarg is used: the generated .cpp files are compiled and linked by meson automatically, which evades any necessity to care about the paths they are generated into. This resolves the issue nicely ...

On the other hand, when using the sources kwarg, the moc tool generates .moc files. Those are meant to be #included in a .cpp file, which makes it necessary to add the correct -Ibuild/dir flag to the correct .cpp file compiling command. I'm not sure whether this is possible or even wanted ...

Any thoughts on this? Should I provide a tiny PR with passing the preserve_path_from argument?

@tristan957
Copy link
Contributor

I think that is very reasonable.

@bruchar1
Copy link
Member

bruchar1 commented Sep 7, 2023

I have an open PR that adds the preserve_paths keyword argument: #11795.

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.

5 participants