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.

Sign up for GitHub

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

Try to resolve issues for x86 Windows #1045

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

Try to resolve issues for x86 Windows #1045

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion deps/build.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,13 @@ try # make sure deps.jl file is removed on error

use_conda = dirname(python) == abspath(Conda.PYTHONDIR)
if use_conda
Conda.add("numpy"; satisfied_skip_solve=true)
if Sys.iswindows() && Sys.WORD_SIZE == 32
# Conda is no longer distributed for 32-bit Windows
# and does not have satisified_skip_solve
Conda.add("numpy")
else
Conda.add("numpy"; satisfied_skip_solve=true)
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't Conda.jl just ignore this flag in that case?

Copy link
Member Author

Choose a reason for hiding this comment

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

In most cases, I would want Conda.jl to error rather than silenting ignoring the keyword. The caller should either anticipate the error or handle the error.

Here, I'm just trying to maintain backwards compatability for a very specific situation, 32-bit Windows, which upstream conda no longer supports.

Copy link
Member

Choose a reason for hiding this comment

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

I'm not 100% convinced that an error is the behavior Conda.add users would want.

The satisfied_skip_solve flag in practice mostly seems like an optional hint to speed things up, as it is used here, and it's a bit ugly that callers of Conda.jl will have to know about and implement the 32-bit windows exception themselves.

Copy link
Member Author

Choose a reason for hiding this comment

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

The critical feature here is not to speed things up, but rather maintain the current version of numpy if already installed.

@MilesCranmer encountered the original issue when working with a conda-forge recipe. Conda-forge tries to depend on the oldest numpy possible for compatibility, but this was inadvertently upgrading numpy to the latest version. This created a binary package cobbling situation.

--satisfied-skip-solve is an important feature supported by all current releases of miniconda and miniforge. At minimum at warning would be needed if this not succeed. Otherwise, we may be doing something the user did not intend.

Honestly, my preferred solution is to stop supporting conda on 32-bit Windows and Python 2.7 entirely due to lack of upstream support. However, this incompatibility occurred without warning.

Copy link
Contributor

Choose a reason for hiding this comment

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

Honestly, my preferred solution is to stop supporting conda on 32-bit Windows and Python 2.7 entirely due to lack of upstream support

I also see this as the best solution.

end
end

(libpython, libpy_name) = find_libpython(python)
Expand Down