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

Various fixes to get basic functionality running on windows #1567

Merged
merged 9 commits into from
Jun 13, 2017

Conversation

mih
Copy link
Member

@mih mih commented Jun 12, 2017

Biggest pieces is to make the global SSHManager optional.

Some proof:

image

@mih
Copy link
Member Author

mih commented Jun 12, 2017

Not so simple, is it....

@yarikoptic
Copy link
Member

is there an immediate demand to make our beast work on windows?

@mih
Copy link
Member Author

mih commented Jun 13, 2017 via email

@mih
Copy link
Member Author

mih commented Jun 13, 2017

NFS failure due to known issue unrelated to these changes.

@codecov-io
Copy link

codecov-io commented Jun 13, 2017

Codecov Report

Merging #1567 into master will decrease coverage by <.01%.
The diff coverage is 92.85%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1567      +/-   ##
==========================================
- Coverage   85.28%   85.27%   -0.01%     
==========================================
  Files         253      253              
  Lines       29181    29197      +16     
==========================================
+ Hits        24886    24899      +13     
- Misses       4295     4298       +3
Impacted Files Coverage Δ
datalad/cmd.py 94.8% <100%> (ø) ⬆️
datalad/distribution/utils.py 97.84% <100%> (+0.07%) ⬆️
datalad/utils.py 90.75% <100%> (+0.05%) ⬆️
datalad/distribution/add.py 94.85% <100%> (ø) ⬆️
datalad/support/network.py 88.72% <100%> (+0.13%) ⬆️
datalad/support/gitrepo.py 87.27% <75%> (-0.12%) ⬇️
datalad/__init__.py 82.19% <85.71%> (-0.67%) ⬇️
datalad/tests/utils.py 87.75% <0%> (-0.18%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 81ef76f...69968a2. Read the comment docs.

@mih
Copy link
Member Author

mih commented Jun 13, 2017

Before this is merged, it needs manual confirmation that it actually works on windows. It should not break other platforms, but the fixes I made are done completely blind -- hope to get to testing tonight.

@yarikoptic
Copy link
Member

if we are supporting windows now, we should establish CI on it, e.g. using appveyor: here is an example https://github.com/duecredit/duecredit/blob/master/appveyor.yml .

@mih
Copy link
Member Author

mih commented Jun 13, 2017

re windows CI: let's wait till at least one test ever passed during manual invocation....

@mih
Copy link
Member Author

mih commented Jun 13, 2017

@bpoldrack @yarikoptic new screenshot -- major progress. create (incl. subdatasets) works, install (incl. subdatasets) works.

In get we are having a windows issue. The German message says that the log file that it tries to remove is still "in use" by another process. I assume this is the downloader.

@mih
Copy link
Member Author

mih commented Jun 13, 2017

If the tests pass, I'll merge this.

@yarikoptic
Copy link
Member

re downloader -- "not sure". that ds000001 could use indeed datalad-archives, but there is also urls per each file which should be just fetched by git-annex curl/wget... you might want to see more of the traceback -- SET DATALAD_EXC_STR_TBLIMIT=20 or so? ;)

Copy link
Member

@yarikoptic yarikoptic left a comment

Choose a reason for hiding this comment

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

in general -- good and worthwhile merging if tests pass right away... but not sure on SSHManager handling as if there is for sure no ssh on windows e.g. http://sshwindows.sourceforge.net/ or may be smth else... not sure though how git/annex handle all the ssh interactions

if win_split[0] and win_split[1]:
# OMG we got something from windows
lgr.log(5, "Detected file ri")
return TYPES['file']
Copy link
Member

Choose a reason for hiding this comment

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

not necessarily from windows -- could be some local masocist ;-)

hopa:~
$> touch c:\\bugi\dugi
2 19687.....................................:Tue 13 Jun 2017 04:53:35 PM EDT:.
hopa:~
$> ls -ld c:\\bugi\dugi
-rw------- 1 yoh yoh 0 Jun 13 16:53 c:\\bugidugi

but the conclusion holds ;)

ssh_manager = SSHManager()
atexit.register(ssh_manager.close, allow_fail=False)
else:
ssh_manager = None
Copy link
Member

Choose a reason for hiding this comment

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

IIRC There could be a cmdline ssh on Windows -- why should we just rule it out at once?

@yarikoptic yarikoptic merged commit 62720dd into datalad:master Jun 13, 2017
@yarikoptic yarikoptic modified the milestone: Release 0.6.1 Jun 25, 2017
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