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

Meson improvements #641

Merged
merged 6 commits into from
Oct 19, 2023
Merged

Meson improvements #641

merged 6 commits into from
Oct 19, 2023

Conversation

YaLTeR
Copy link
Contributor

@YaLTeR YaLTeR commented Oct 19, 2023

A number of meson improvements from my work to instrument Mutter and its dependencies. See individual commits.

The header install dir fix makes it work without errors, but it's still a bit ugly because it installs stuff into /usr/include/client/ and /usr/include/common/. Ideally everything should go under /usr/include/tracy/, but that breaks some include paths relying on ../. I might try to figure it out later.

@wolfpld
Copy link
Owner

wolfpld commented Oct 19, 2023

Can you add a github action that would run the meson build path? It would make testing the changes easier.

@YaLTeR
Copy link
Contributor Author

YaLTeR commented Oct 19, 2023

Yeah, let me try.

@YaLTeR YaLTeR force-pushed the meson-improvements branch from bba7a2d to f5bab45 Compare October 19, 2023 09:54
@@ -35,6 +35,8 @@ jobs:
run: make -j`nproc` -C import-chrome/build/unix debug release
- name: Library
run: make -j`nproc` -C library/unix debug release
- name: Library (meson)
run: meson setup build && meson compile -C build
Copy link
Owner

Choose a reason for hiding this comment

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

This probably needs to be excluded on macos.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Owner

Choose a reason for hiding this comment

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

Never mind then. Maybe we should also do ninja install and then publish the artifacts?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought about this a bit, especially considering that I do package this .so for my instrumented mutter repo. However, I don't think it makes much sense to do generally, because in practice you do want to set different defines based on the use-case (i.e. I set TRACY_ONLY_LOCALHOST for anything I send to others, yet I leave it off for my own machines), and a built library supports only a single set of defines. If it was configured at runtime instead, then it would make more sense to publish it IMO.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess TRACY_ONLY_LOCALHOST is not a good example, TRACY_MANUAL_LIFETIME is a better one that needs explicit support from the profiled application.

Copy link
Owner

Choose a reason for hiding this comment

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

If it was configured at runtime instead, then it would make more sense to publish it IMO.

My intention was more to see what the install job does, if all the files are where they should be, etc. It may not be apparent just from looking at the build script.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hm, then could do an install with a prefix set to local dir. Meson prints all files it installs.

Copy link
Owner

Choose a reason for hiding this comment

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

Sure.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@YaLTeR YaLTeR force-pushed the meson-improvements branch from f5bab45 to ed486bf Compare October 19, 2023 10:39
@wolfpld wolfpld merged commit 7f5cfdf into wolfpld:master Oct 19, 2023
5 checks passed
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.

2 participants