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

Windows compatibility #48

Merged
merged 30 commits into from
Nov 8, 2018
Merged

Windows compatibility #48

merged 30 commits into from
Nov 8, 2018

Conversation

dsludwig
Copy link
Contributor

@dsludwig dsludwig commented Sep 24, 2018

Replaces #46

Includes an appveyor configuration for Windows CI: https://ci.appveyor.com/project/dsludwig/conda-pack/

There's an outstanding issue when the archives are extracted on a case-sensitive file-system, but I didn't want to add that complexity for what is likely not a common case. I've included an extensive comment that explains the issue in more detail.

@jcrist
Copy link
Collaborator

jcrist commented Sep 26, 2018

Before I review (hopefully tomorrow), could you quick fix the merge conflicts that have popped up? This will make it easier to review, and get testing setup on the main branch. Thanks for all your work here.

@jcrist jcrist mentioned this pull request Sep 27, 2018
@dsludwig dsludwig changed the title [WIP] Windows compat Windows compatibility Oct 2, 2018
Copy link
Contributor Author

@dsludwig dsludwig left a comment

Choose a reason for hiding this comment

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

@jcrist I've resolved the latest merge conflicts with master.

Passing build on Windows is here: https://ci.appveyor.com/project/dsludwig/conda-pack/build/1.0.13

conda_pack/core.py Show resolved Hide resolved
conda_pack/scripts/windows/activate.bat Show resolved Hide resolved
conda_pack/tests/test_core.py Show resolved Hide resolved
@certik
Copy link

certik commented Oct 12, 2018

@dsludwig, the Travis tests fail. Do you know how to fix that?

@dsludwig
Copy link
Contributor Author

Thanks for the ping @certik, I found the problem in the tests. It's passing now!

Copy link

@certik certik left a comment

Choose a reason for hiding this comment

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

The changes on linux seem fine to me, and the linux tests pass on Travis, so I would say it's ok. If something broke, we should improve the linux tests.

Regarding the changes on Windows, they look ok as the first iteration, and so I suggest to merge it, so that the appveyor tests get enabled, and so we can then send subsequent PRs and they will get tested on Windows.

@jcrist would you please mind reviewing it and merging?

conda_pack/core.py Show resolved Hide resolved
@certik
Copy link

certik commented Oct 15, 2018

@jcrist ping.

@mcg1969
Copy link
Contributor

mcg1969 commented Oct 20, 2018

AppVeyor is passing here, but holy cow is it slow. pip itself seems to be a major culprit in the slowness, and I'm not sure why.

@mcg1969
Copy link
Contributor

mcg1969 commented Oct 20, 2018

OK, I spoke to soon; @dsludwig, it looks like it's not passing with Python 3

@mingwandroid
Copy link

Yes

Ah ok then sorry for the noise. Do what you will with them (within reason!)

@mcg1969 mcg1969 closed this Oct 22, 2018
@mcg1969 mcg1969 deleted the windows-compat branch October 22, 2018 21:40
@mcg1969 mcg1969 restored the windows-compat branch October 22, 2018 21:41
@mcg1969 mcg1969 reopened this Oct 22, 2018
@mcg1969
Copy link
Contributor

mcg1969 commented Oct 22, 2018

Per @jcrist's request, #54 has been incorporated.

@mcg1969
Copy link
Contributor

mcg1969 commented Oct 23, 2018

Last test failure was a flake issue. Functional tests passing.

@mcg1969
Copy link
Contributor

mcg1969 commented Oct 23, 2018

Now the only test that doesn't run on Windows is test_keyboard_interrupt. For test_formats, I copied the relevant files/trees instead of symlink to them, so the result is effectively the same as if I did check(..., False).

@jcrist jcrist merged commit a5ab91a into conda:master Nov 8, 2018
@jcrist
Copy link
Collaborator

jcrist commented Nov 8, 2018

@dsludwig and @mcg1969, thanks for all the work here, and for being patient with my slow review time. This is in.

@certik
Copy link

certik commented Nov 8, 2018

Thanks everybody!

@mcg1969 mcg1969 deleted the windows-compat branch November 8, 2018 16:16
@fdabek1
Copy link

fdabek1 commented Nov 8, 2018

@jcrist Any idea when this will be released? I am waiting eagerly for the next release on conda-forge so that I can utilize this without installing from source.

@jcrist
Copy link
Collaborator

jcrist commented Nov 8, 2018

I expect to issue a release sometime in the next week.

@github-actions
Copy link

Hi there, thank you for your contribution!

This pull request has been automatically locked because it has not had recent activity after being closed.

Please open a new issue or pull request if needed.

Thanks!

@github-actions github-actions bot added the locked [bot] locked due to inactivity label Mar 22, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 22, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
locked [bot] locked due to inactivity
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants