-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
[win] Fix source build without pre-built deps #45712
base: master
Are you sure you want to change the base?
Conversation
And use TAB in makefile
@@ -5,6 +5,12 @@ ifneq ($(USE_BINARYBUILDER_P7ZIP),1) | |||
$(SRCCACHE)/p7zip-$(P7ZIP_VER).tar.gz: | $(SRCCACHE) | |||
$(JLDOWNLOAD) $@ https://github.com/jinfeihan57/p7zip/archive/refs/tags/v$(P7ZIP_VER).tar.gz | |||
|
|||
P7ZIP_BUILD_OPTS := $(MAKE_COMMON) | |||
# Build with cygwin, not use cross build. | |||
ifeq (,$(findstring CYGWIN,$(BUILD_OS))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems unlikely this should care about the BUILD_OS
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it's a workaround for cygwin.
I'm not sure if there is a better solution.
It's incredibly frustrating to build p7zip on mingw, so instead we just redistribute 7z
https://github.com/JuliaPackaging/Yggdrasil/blob/b2e0c2c5851b71230fb7170f74d773393ce37f80/P/p7zip/build_tarballs.jl#L21
In my case, on Windows, build p7zip with gcc
on cygwin, works well.
But when cross build p7zip with x86_64-w64-mingw32-gcc
it fails.
Co-authored-by: Jameson Nash <[email protected]>
Co-Authored-By: Jameson Nash <[email protected]>
Some suggestions:
for Makefile-based dep.
|
Line 58 in 7838124
Yes, we can add those flags to
But it will break offline build steps.
Line 238 in 7838124
Well hidden
OK, done. And I'm testing cross build on WSL2. |
Co-Authored-By: Y. Yang <[email protected]>
We need to pass `OS` to `make install`
I would like to split this pr into a series of smaller pr's for easier review: |
fix #45645, #45745