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 custom build directory #221

Merged
merged 10 commits into from
Aug 15, 2024
Merged

Conversation

stefanv
Copy link
Member

@stefanv stefanv commented Jul 13, 2024

Closes #171

  • Add -C flag to meson commands
  • Add tests and verify that compilation happens in right place

@stefanv stefanv added the type: Enhancement New feature or request label Jul 13, 2024
@stefanv stefanv marked this pull request as ready for review July 17, 2024 21:22
@stefanv
Copy link
Member Author

stefanv commented Jul 17, 2024

@lagru Ready for testing.

Copy link
Member

@lagru lagru left a comment

Choose a reason for hiding this comment

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

Thanks @stefanv! I played around with it locally and came up with a few limitations. Would be good to iterate further on the API and behavior. :)

Comment on lines 286 to 290
This feature is useful in combination with a shell alias, e.g.:

$ alias spin-clang="spin -C build-clang"

Which can then be used to build (`spin-clang build`), to test (`spin-clang test ...`), etc.
Copy link
Member

Choose a reason for hiding this comment

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

The name build-clang might be a bit misleading. To me it implies that this feature can somehow be used to choose a different compiler (clang). As I understand it, you'd need to update the underlying "build" command / configuration which would then apply regardless of build directory? Or am I missing something?


This feature is useful in combination with a shell alias, e.g.:

$ alias spin-clang="spin -C build-clang"
Copy link
Member

Choose a reason for hiding this comment

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

spin -C ${SOME_DIR} command seems to fail. I think this is because the option is added ad subcommand level. So this alias won't work currently. I think the option would need to be added to the top-level spin command and then passed down via the context.

Copy link
Member Author

Choose a reason for hiding this comment

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

Right, may have to rethink that. We don't have global context in spin.

spin/cmds/meson.py Outdated Show resolved Hide resolved
@stefanv
Copy link
Member Author

stefanv commented Jul 18, 2024

@lagru Please take another look; the build directory can now be absolute, and can be set using SPIN_BUILD_DIR, which is helpful for creating an alias. I also updated the alias to be a better example, showing how to set the compiler to clang and build into a different build directory using clang.

Copy link
Member

@lagru lagru left a comment

Choose a reason for hiding this comment

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

Thanks. Absolute, relative and nested-relative paths seem to work perfectly now. I'm not familiar with the code base so take my approval with a grain of salt. ;)

@stefanv stefanv merged commit d624569 into scientific-python:main Aug 15, 2024
20 checks passed
@oscarbenjamin
Copy link

Nice!

I was just about to suggest this feature. I just gave it a quick er... spin and it seems to work exactly as I wanted.

One suggestion though:

When you run meson setup build-dir it adds a .gitignore to the build directory. I already have a .gitignore entry for the build-install directory but if I do:

meson setup build-release -Dbuildtype=release
spin -C build-release run ...

Then the build-release-install directory is created but it does not have any .gitignore file. Perhaps spin could create the directory and add the .gitignore file before running meson install.

@stefanv
Copy link
Member Author

stefanv commented Sep 3, 2024

@oscarbenjamin What you request sounds very reasonable, but it's already the way my setup behaves:

$ spin build -C build-xyz && ls -al build-xyz/.gitignore
-rw-r--r--. 1 stefan stefan 92 Sep  2 21:05 build-xyz/.gitignore

I see the same behavior when adding the -Dbuildtype=release flag.

@oscarbenjamin
Copy link

it's already the way my setup behaves:

I've just checked again and confirm that the .gitignore is not added here:

$ spin build -C build-foo -- --pkg-config-path=.local/lib/pkgconfig -Dadd_flint_rpath=true
...
$ git status
On branch pr_nmod_ctx
Your branch is up-to-date with 'origin/pr_nmod_ctx'.

Untracked files:
  (use "git add <file>..." to include in what will be committed)
	build-abc-install/
	build-foo-install/
	build-xyz-install/

nothing added to commit but untracked files present (use "git add" to track)

Is it meson that adds it in the meson install step?

I didn't expect that meson install would do that so it didn't seem surprising that the .gitignore was not being added.

@jarrodmillman jarrodmillman added this to the 0.12 milestone Sep 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: Enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Facilitate parallel build directories for meson-enabled commands?
4 participants