-
Notifications
You must be signed in to change notification settings - Fork 3.7k
[FFI][CMAKE] Add missing download path for libbacktrace #18246
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
Conversation
fdf72c1 to
95da9f6
Compare
|
At this point (CI not fully finished) MACOS is green now with upstream backtrace. |
|
actually we may not need this as long as the bakctrace is intialize by git submodule update --init |
It looked (and still looks) gone for me, like this: Ahh, it changed home (i did not noticed this) ... I am sorry then, you are right ! I maintain since 4-5 years aautomated rpm packagebuild that use that path desribed earlier (selective explicit path): But beside that, if we now reached here accidentally (and fixed MacOS build) IMHO it is easier and cleaner to maintain from cmake "way" than from "git submodule" way. The advantage with cmake is that it can download source conditionally on the case it cannot be found locally (precompiled). This can be reverted if you decide. |
|
one issue was that the autodownload can cause issues for repeated compilation (@MasterJH5574 reported that even if there is no source change, libbacktrace may get recompiled). also when we ship pip ideally there should be not extra download. would be good to revert this for now. |
|
but it is probbaly good idea to update libbacktrace to use upstream instead in git submodule |
…)" This reverts commit dd1e3f8.
* Revert the URL out from cmake for libbacktrace * Switch git submodule to upstream HEAD instead As per discussed here #18246 (comment), this reverts in favour of git submodule way. As per finding in the same discuss the upstream [already](https://github.com/ianlancetaylor/libbacktrace/blob/793921876c981ce49759114d7bb89bb89b2d3a2d/macho.c#L1273-L1275) incorporates [the one patch](ianlancetaylor/libbacktrace@master...tlc-pack:libbacktrace:master) used, and MacOS works fine.
…he#18249) * Revert the URL out from cmake for libbacktrace * Switch git submodule to upstream HEAD instead As per discussed here apache#18246 (comment), this reverts in favour of git submodule way. As per finding in the same discuss the upstream [already](https://github.com/ianlancetaylor/libbacktrace/blob/793921876c981ce49759114d7bb89bb89b2d3a2d/macho.c#L1273-L1275) incorporates [the one patch](ianlancetaylor/libbacktrace@master...tlc-pack:libbacktrace:master) used, and MacOS works fine.
…he#18249) * Revert the URL out from cmake for libbacktrace * Switch git submodule to upstream HEAD instead As per discussed here apache#18246 (comment), this reverts in favour of git submodule way. As per finding in the same discuss the upstream [already](https://github.com/ianlancetaylor/libbacktrace/blob/793921876c981ce49759114d7bb89bb89b2d3a2d/macho.c#L1273-L1275) incorporates [the one patch](ianlancetaylor/libbacktrace@master...tlc-pack:libbacktrace:master) used, and MacOS works fine.
…he#18249) * Revert the URL out from cmake for libbacktrace * Switch git submodule to upstream HEAD instead As per discussed here apache#18246 (comment), this reverts in favour of git submodule way. As per finding in the same discuss the upstream [already](https://github.com/ianlancetaylor/libbacktrace/blob/793921876c981ce49759114d7bb89bb89b2d3a2d/macho.c#L1273-L1275) incorporates [the one patch](ianlancetaylor/libbacktrace@master...tlc-pack:libbacktrace:master) used, and MacOS works fine.
This PR adds the missing URL for cmake's extrenal project (auto)-builder for
libbacktrace.Not sure about releng FFI intention/migration, but I added
the last seenupstream URLin PR#18226libbacktrace:UPDATE:
Due to MaCOS CI failure, replaced to uptream repo latest instead (repushed this PR).
Seems that upstream libbacktrace already contain the single patch from tlc-pack.