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

Use Compat.Sys.BINDIR so that BinDeps can work with system images #359

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rofinn
Copy link

@rofinn rofinn commented Mar 21, 2018

No description provided.

@@ -122,9 +118,9 @@ if Sys.KERNEL == :FreeBSD
end

if Compat.Sys.iswindows()
const exe7z = joinpath(_HOME, "7z.exe")
Copy link
Contributor

Choose a reason for hiding this comment

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

it's possible other packages may have been referring to this constant

Copy link
Author

Choose a reason for hiding this comment

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

I really hope not since this would only existing on windows. I could undo this without causing problems for us, but I'm pretty sure creating system images on windows won't work correctly.

Copy link
Contributor

Choose a reason for hiding this comment

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

creating system images on windows won't work correctly

Why's that? What is this fixing anyway?

Copy link
Author

@rofinn rofinn Mar 27, 2018

Choose a reason for hiding this comment

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

The issue is that JULIA_HOME (or I guess now Sys.BINDIR) doesn't exist at system image build time, which will cause functions that depend on the compiled constant to fail at runtime. Since I don't use windows I'm not particularly bothered by leaving this in the windows if block, but this could cause problems for windows user who want to make system images.

@rofinn
Copy link
Author

rofinn commented Apr 28, 2018

Ping? Would folks like me to prioritize non-exported constants on Windows over building system images on windows?

@KristofferC
Copy link
Member

Being able to build sysimgs on Windows seems like a good thing :)

@tkelman
Copy link
Contributor

tkelman commented Apr 28, 2018

it's not possible to preserve the compatibility and have both?

@rofinn
Copy link
Author

rofinn commented Apr 28, 2018

Hmmm, the only way we can keep the windows constant is by setting it in an __init__ function?

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