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

[ittapi] Check out branch only if ITTAPI is cloned #18

Closed
wants to merge 1 commit into from

Conversation

kpamnany
Copy link

When building from a tarball (as opposed to an online build), ittapi is not a repository so we shouldn't try to check out a branch.

@kpamnany kpamnany requested a review from vchuravy June 20, 2023 19:11
@kpamnany
Copy link
Author

This didn't work correctly with the changes I made to deps/llvm.mk. Fix coming.

Use `${ITTAPI_SOURCE_DIR}` rather than `${ITTAPI_SOURCE_DIR}/ittapi` for
the ITTAPI location.

Check out the `${ITTAPI_GIT_TAG}` only if ITTAPI is cloned because when
building from a tarball (as opposed to an online build),
`${ITTAPI_SOURCE_DIR}` is not a repository so we shouldn't try to check
out a branch.
@kpamnany
Copy link
Author

Okay, this seems to work (from here).

Alternatively it could be put in deps/patches (as in the linked post above) rather than merged in here. Up to you guys.

@vchuravy
Copy link
Member

Ideally we would submit this first upstream.
In the mean-time carrying it as an offline patch is fine,
I just want to avoid rebuilding LLVM for a build-system change.

@vchuravy
Copy link
Member

LLVM doesn't yet use GitHub pull-request, but hopefully will starting this fall,.

https://llvm.org/docs/Contributing.html#id9 for how to submit a patch.

@giordano
Copy link

Closing this PR as this we moved forward with our LLVM version, and this PR is also targeting the wrong branch, should use one of our julia-release/N.x branch, not main.

@giordano giordano closed this Oct 13, 2024
@giordano giordano deleted the kp/fix-ittapi branch October 13, 2024 14:31
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.

3 participants