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

Allow path separators in output kwargs #8521

Closed
wants to merge 1 commit into from

Conversation

tristan957
Copy link
Contributor

@tristan957 tristan957 commented Mar 11, 2021

Path separators in output kwargs is very important for allowing Meson to wrap other tooling/compilers. Take for instance Meson wrapping Maven. This may sound kind of crazy, but imagine a C library and you are writing JNI bindings for it. There are essentially 2 build artifacts that you need to generate: a JNI library written in C and a JAR written in Java. In my case, errors in the C library exist as what are essentially errno values. I decided to pull in a dependency from Maven Central which provides errno values in Java-land. Manual management of the class path can be quite difficult and tedious among other things when reaching for javac directly. I decided to use Maven to compile the Java sources and create a JAR. What I essentially want is a one step process to build both my JNI library and the JAR (ninja -C build). Maven produces many artifacts in many different folders. Just off the top of my head, if you didn't care about any of the other build artifacts, Maven produces the JAR in the root of its build directory, and *.class files in the a subdir called classes. In order to properly model Maven in Meson, I need access to path separators in output kwargs. I have tested this patch locally on my system, and it works great.

Not entirely sure what the best way to add a test is.

Ninja: 1.10
Meson: master

Fixes #2320

@tristan957
Copy link
Contributor Author

I see @nirbheek did this quite a while back: 7c15837.
Added some extra protections as well, which this patch should since it is pretty much the exact same thing, and I agree with his restrictions. Would love to generate some discussion around this.

@tristan957
Copy link
Contributor Author

tristan957 commented Mar 11, 2021

I understand why this restriction existed, but if Meson is to grow to something larger, this is a necessary update. Not every compiler/generator/tool has the ability to be told where artifacts should go, unlike C compilers for instance.

Meson's purpose on my project is to not only support compiling my C library, but also a meta-workflow where the main library, and its bindings/other projects can all be built with Meson, so when I change the public interface, I know how how I am affecting an entire ecosystem of software.

hse
├── lib
│   ├── ex.c
│   └── meson.build
├── meson.build
└── subprojects
    ├── hse-cpp.wrap
    ├── hse-go.wrap
    ├── hse-java.wrap
    ├── hse-python.wrap
    └── hse-rust.wrap

@jpakkane
Copy link
Member

I understand why this restriction existed, but if Meson is to grow to something larger, this is a necessary update. Not every compiler/generator/tool has the ability to be told where artifacts should go, unlike C compilers for instance.

Having this limitation is a good thing and it makes things easier and simpler for the vast majority of people and use cases while making things slightly inconvenient for those who use broken tools. Switching it makes things worse for the majority who do it right for the benefit of a minority that does things wrong. This will not be accepted. A different approach would be to make a higher level interface (like how unstable_external_project works) that does what people want rather than break invariants so that people need to then fix things by hand in their build files. That is both fragile and tedious.

@tristan957
Copy link
Contributor Author

You can't properly model dependency graphs without path separators in so many tools. I don't understand what would break here. Previously path separators were not allowed, and now they are in this PR. What would break? The people that need the feature will use it, the people that don't won't.

@eli-schwartz
Copy link
Member

eli-schwartz commented Mar 11, 2021

Removing a design restriction requires more rationale than merely coding the implementation.

This PR is inherently unmergeable on the grounds that the discussion in the linked issue does not include the lead meson developer (@jpakkane) making the philosophical call to change his mind about this.

Closing this PR to prevent discussion about "why" from ending up split across two different github tickets. Further discussion shall be continued on the linked issue.

@mesonbuild mesonbuild locked and limited conversation to collaborators Mar 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow configure_file, custom_target and generator output file to subdirectories under builddir
3 participants