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

Makefile: run the cd commands in a subshell #5073

Closed
wants to merge 1 commit into from
Closed

Makefile: run the cd commands in a subshell #5073

wants to merge 1 commit into from

Conversation

konimex
Copy link
Contributor

@konimex konimex commented Mar 11, 2020

In bmake, if the directory is changed (with cd or anything else), bmake won't return to the "root directory" on the next command (in the same Makefile rule). This commit runs the cd command in a subshell so it would work in bmake. This has been applied in

tidy:
so I don't see why it shouldn't apply anywhere else.

Logs:
Before my commit
After my commit

In bmake, if the directory is changed (with cd or anything else), bmake
won't return to the "root directory"  on the next command (in the same
Makefile rule). This commit runs the cd command in a subshell so it
would work in bmake.
@bagder
Copy link
Member

bagder commented Mar 11, 2020

Only one question: should we prefer $(MAKE) -C $dir to (cd $dir && $(MAKE)) ? Looking at our current makefile set, we use a mix all over...

@bagder bagder added the build label Mar 11, 2020
@konimex
Copy link
Contributor Author

konimex commented Mar 11, 2020

If POSIX is of any concern, then the latter. However, both GNU make and BSD make support -C, so I don't know if there are any more make(1) implementations that don't implement -C.

@bagder
Copy link
Member

bagder commented Mar 11, 2020

Thanks! So let's go with the cd approach. Will merge!

@bagder bagder closed this in 0cc66ff Mar 11, 2020
@konimex konimex deleted the bmake branch March 11, 2020 15:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

2 participants