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

Faciliate usage of jom #932

Merged
merged 2 commits into from
Jun 22, 2022
Merged

Faciliate usage of jom #932

merged 2 commits into from
Jun 22, 2022

Conversation

jheaff1
Copy link
Collaborator

@jheaff1 jheaff1 commented Jun 20, 2022

The jom tool (a fork of nmake which supports parallel builds) does not
support the "-C dir" arg. Given that the build script generated by
rules_foreign_cc changes the working directory to $BUILD_TMPDIR before
invoking make/nmake/jom, there is no need to add the "-C $BUILD_TMPDIR"
arg. This commit removes the addition of the "-C $BUILD_TMPDIR" arg so
that jom can be used as a make variant.

The jom tool (a fork of nmake which supports parallel builds) does not
support the "-C <dir>" arg. Given that the build script generated by
rules_foreign_cc changes the working directory to $BUILD_TMPDIR before
invoking make/nmake/jom, there is no need to add the "-C $BUILD_TMPDIR"
arg. This commit removes the addition of the "-C $BUILD_TMPDIR" arg so
that jom can be used as a make variant.
@jheaff1 jheaff1 marked this pull request as ready for review June 20, 2022 17:53
Copy link
Member

@jsharpe jsharpe left a comment

Choose a reason for hiding this comment

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

I'm fine with this - the reason for introducing the -C argument was to try to facilitate building from the root of the execroot without changing directories but it doesn't work in a useful way to fix resolving relative paths.

@jsharpe jsharpe enabled auto-merge (squash) June 22, 2022 19:28
@jsharpe jsharpe merged commit 3b0cf7d into bazel-contrib:main Jun 22, 2022
@jheaff1 jheaff1 deleted the facilitate_jom branch June 22, 2022 20:28
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