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

Take a step toward complete BB domination #33125

Merged
merged 1 commit into from
Sep 18, 2019

Conversation

staticfloat
Copy link
Member

Provide libexpat, libz and p7zip (true 7z for Windows) on all platforms. This provides a number of benefits:

  • Significantly reduces the amount of work in win-extras, which I've always thought was an annoying wart in our build system.

  • Provides a consistent 7z executable on all platforms, which can be picked up by Pkg/BinaryProvider for consistent unzipping/untarring/ungzipping on all platforms. Note that on Windows, 7z is a repackaged binary from sourceforge and is the "full" 7z, whereas on all other platforms it is a p7zip build of the standalone 7za executable with all relevant patches applied. There are some format capability differences, but for the important things it should be fine.

  • We kind of half-heartedly guaranteed to packages like WinRPM (through LibExpat.jl) that libexpat would always be available. This is inconsistently true across platforms, so let's fix that by just shipping it everywhere just like any other dependency.

@staticfloat staticfloat force-pushed the sf/complete_bb_domination branch from 655e822 to 4c71f14 Compare August 30, 2019 22:58
@staticfloat staticfloat requested a review from vtjnash August 30, 2019 23:36
staticfloat added a commit to JuliaCI/julia-buildbot that referenced this pull request Aug 30, 2019
This will not be needed anymore if we bundle `7z` with BB, as attempted
in JuliaLang/julia#33125
@StefanKarpinski
Copy link
Member

StefanKarpinski commented Sep 9, 2019

Unless we actually need XML parsing for Pkg, I’d prefer not to include libexpat. Can’t it be a normal BB-provided library?

@staticfloat
Copy link
Member Author

The XML parsing isn't for Pkg, it's for WinRPM. I think we could get around this by making a 1.3-only release of WinRPM that uses Pkg.Artifacts to download libexpat. @vtjnash does that seem reasonable to you?

@staticfloat staticfloat force-pushed the sf/complete_bb_domination branch from bfe85a7 to 6508d7d Compare September 10, 2019 03:44
@staticfloat
Copy link
Member Author

With JuliaIO/LibExpat.jl#99 passing tests, I took libexpat out of this PR, so it only provides p7zip and libz.

@staticfloat staticfloat changed the title Take a step toward complete BB domination [WIP] Take a step toward complete BB domination Sep 10, 2019
@staticfloat staticfloat changed the title [WIP] Take a step toward complete BB domination Take a step toward complete BB domination Sep 12, 2019
@staticfloat
Copy link
Member Author

This is ready to merge.

else
$(error no win-extras target for ARCH=$(ARCH))
endif
@$(MAKE) -C $(BUILDROOT)/deps install-p7zip
Copy link
Member

Choose a reason for hiding this comment

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

this recursive calls to MAKE is race-y, so it's generally a bad idea—but also unnecessary (since the julia-deps target already does it)?

Copy link
Member Author

Choose a reason for hiding this comment

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

win-extras does not depend on julia-deps, nor do we want it to. We don't want to e.g. install LLVM when you run win-extras, we only want to install p7zip; unfortunately we don't have fine-grained dependency control, so the next best thing is recursive make. install-p7zip is a simple enough operation that I think it's not that bad. julia-deps itself is just a (much larger) recursive make call anyway.

Copy link
Member

Choose a reason for hiding this comment

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

eh, ok, it's wrong but not too bad as you say

Copy link
Member Author

Choose a reason for hiding this comment

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

I think the only way this could be done "right" is if we were to include deps/Makefile from Makefile, giving us access to all those targets.

@staticfloat
Copy link
Member Author

Unless I get another review response from Jameson, I'm going to merge this at the end of today.

deps/zlib.mk Show resolved Hide resolved
@staticfloat
Copy link
Member Author

Talked this over in the Pkg call; we are going to implement support for USE_BINARYBUILDER=0 on non-Windows platforms, since that's where people care about it most.

@staticfloat staticfloat force-pushed the sf/complete_bb_domination branch 2 times, most recently from 83e96e3 to 031cdae Compare September 17, 2019 22:49
This provides a number of benefits:

* Significantly reduces the amount of work in win-extras, which I've
always thought was an annoying wart in our build system.

* Provides a consistent 7z executable on all platforms, which can be
picked up by Pkg/BinaryProvider for consistent
unzipping/untarring/ungzipping on all platforms. Note that on Windows,
7z is a repackaged binary from sourceforge and is the "full" 7z, whereas
on all other platforms it is a p7zip build of the standalone 7za
executable with all relevant patches applied. There are some format
capability differences, but for the important things it should be fine.
@staticfloat staticfloat force-pushed the sf/complete_bb_domination branch from 031cdae to bbf7310 Compare September 17, 2019 22:49
@staticfloat
Copy link
Member Author

staticfloat commented Sep 17, 2019

Updated PR: we now provide build recipes for libz and p7zip for those that wish to disable BB downloading via either USE_BINARYBUILDER=0 or more selectively via USE_BINARYBUILDER_ZLIB=0/USE_BINARYBUILDER_P7ZIP=0. The p7zip build is expected to fail on Windows; you have no choice but to use BB or set USE_SYSTEM_P7ZIP on Windows.

I have tested that this builds p7zip and libz without BB on MacOS x86_64, Linux x86_64 and AArch64, so I'm satisfied that this is working reasonably well. Note that I did not take the time to tune compiler flags for 7z (it has a very strange, very manual process to do this) so from-source builds of 7z might be a little bit slower than the BB builds. If that's important to you, I forward you to the advice I once heard from 0bit-One: Use The Binaries, Luke.

@KristofferC KristofferC mentioned this pull request Sep 18, 2019
18 tasks
@staticfloat staticfloat merged commit b6ddd87 into master Sep 18, 2019
@staticfloat staticfloat deleted the sf/complete_bb_domination branch September 18, 2019 16:42
KristofferC pushed a commit that referenced this pull request Sep 22, 2019
This provides a number of benefits:

* Significantly reduces the amount of work in win-extras, which I've
always thought was an annoying wart in our build system.

* Provides a consistent 7z executable on all platforms, which can be
picked up by Pkg/BinaryProvider for consistent
unzipping/untarring/ungzipping on all platforms. Note that on Windows,
7z is a repackaged binary from sourceforge and is the "full" 7z, whereas
on all other platforms it is a p7zip build of the standalone 7za
executable with all relevant patches applied. There are some format
capability differences, but for the important things it should be fine.

(cherry picked from commit b6ddd87)
@omus
Copy link
Member

omus commented Sep 24, 2019

Provides a consistent 7z executable on all platforms

What's the recommended practise for using the 7z executable? On the current nightly build of Julia (1.4.0-DEV.194) I can't seem to locate the executable.

@staticfloat
Copy link
Member Author

staticfloat commented Sep 24, 2019 via email

@omus
Copy link
Member

omus commented Sep 24, 2019

On the Julia nightly for Windows 64-bit the joinpath(Sys.BINDIR, "..", "libexec") folder is present but is empty.

@ararslan
Copy link
Member

7z fails to build on FreeBSD with USE_BINARYBUILDER=0.

@omus
Copy link
Member

omus commented Nov 4, 2019

Related: #33592

eggheaded added a commit to eggheaded/WinRPM.jl that referenced this pull request Dec 5, 2019
vchuravy pushed a commit to JuliaLang/LazyArtifacts.jl that referenced this pull request Oct 2, 2023
This provides a number of benefits:

* Significantly reduces the amount of work in win-extras, which I've
always thought was an annoying wart in our build system.

* Provides a consistent 7z executable on all platforms, which can be
picked up by Pkg/BinaryProvider for consistent
unzipping/untarring/ungzipping on all platforms. Note that on Windows,
7z is a repackaged binary from sourceforge and is the "full" 7z, whereas
on all other platforms it is a p7zip build of the standalone 7za
executable with all relevant patches applied. There are some format
capability differences, but for the important things it should be fine.

(cherry picked from commit 8df4902)
vchuravy pushed a commit to JuliaPackaging/LazyArtifacts.jl that referenced this pull request Oct 2, 2023
This provides a number of benefits:

* Significantly reduces the amount of work in win-extras, which I've
always thought was an annoying wart in our build system.

* Provides a consistent 7z executable on all platforms, which can be
picked up by Pkg/BinaryProvider for consistent
unzipping/untarring/ungzipping on all platforms. Note that on Windows,
7z is a repackaged binary from sourceforge and is the "full" 7z, whereas
on all other platforms it is a p7zip build of the standalone 7za
executable with all relevant patches applied. There are some format
capability differences, but for the important things it should be fine.
vchuravy pushed a commit to JuliaLang/Distributed.jl that referenced this pull request Oct 6, 2023
This provides a number of benefits:

* Significantly reduces the amount of work in win-extras, which I've
always thought was an annoying wart in our build system.

* Provides a consistent 7z executable on all platforms, which can be
picked up by Pkg/BinaryProvider for consistent
unzipping/untarring/ungzipping on all platforms. Note that on Windows,
7z is a repackaged binary from sourceforge and is the "full" 7z, whereas
on all other platforms it is a p7zip build of the standalone 7za
executable with all relevant patches applied. There are some format
capability differences, but for the important things it should be fine.

(cherry picked from commit 16225e6)
vchuravy pushed a commit to JuliaLang/Test.jl that referenced this pull request Oct 7, 2023
This provides a number of benefits:

* Significantly reduces the amount of work in win-extras, which I've
always thought was an annoying wart in our build system.

* Provides a consistent 7z executable on all platforms, which can be
picked up by Pkg/BinaryProvider for consistent
unzipping/untarring/ungzipping on all platforms. Note that on Windows,
7z is a repackaged binary from sourceforge and is the "full" 7z, whereas
on all other platforms it is a p7zip build of the standalone 7za
executable with all relevant patches applied. There are some format
capability differences, but for the important things it should be fine.

(cherry picked from commit 358c406)
Keno pushed a commit that referenced this pull request Jun 5, 2024
This provides a number of benefits:

* Significantly reduces the amount of work in win-extras, which I've
always thought was an annoying wart in our build system.

* Provides a consistent 7z executable on all platforms, which can be
picked up by Pkg/BinaryProvider for consistent
unzipping/untarring/ungzipping on all platforms. Note that on Windows,
7z is a repackaged binary from sourceforge and is the "full" 7z, whereas
on all other platforms it is a p7zip build of the standalone 7za
executable with all relevant patches applied. There are some format
capability differences, but for the important things it should be fine.
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.

6 participants