Path issue for diffs on Windows

[jamesjnadeau]

I searched for other issues like this, but couldn't find any.

Description

I'm not able to get the diff tool to work on Windows. When trying to view a diff, the DiffWidget component displays an error. I believe this is related to paths being improperly passed to the backend for diffs when using windows paths.

I believe the function call here is causing the issue:
https://github.com/jupyterlab/jupyterlab-git/blob/master/src/components/diff/DiffWidget.tsx#L38
but I'm new here, so take that with a grain of salt 🤷‍♂

Reproduce

1. Go to a repo that has changes in it
2. Click on the diff icon for a file that has been changed to view the diff
3. See error Failed to fetch diff with error:JSON.parse: unexpected character at line 1 column 1 of the JSON data

Expected behavior

I would expect to be able to view this diff on the windows platform.

Upon further inspection, I can see the following parameters passed in a Post to my lab server:

{
  "file_path":"../C:/path_to_a notebook.ipynb",
  "ref_local":{"git":"HEAD"},
  "ref_remote":{"special":"WORKING"}
}

making the request without ../ in file_path appears to fix the issue.

I'm familiar with react but I'm not sure where in this project this is getting messed up, and I"m not set up to easily develop on this(although I'm totally willing to figure it out if someone can point me to some links to get up and running)

Context

• Python package version:

conda list jupyterlab-git
# packages in environment at C:\Users\username\AppData\Local\Continuum\miniconda3:
#
# Name                    Version                   Build  Channel
jupyterlab-git            0.8.2                      py_0    conda-forge

• Extension version:

jupyter labextension list
JupyterLab v1.1.4
Known labextensions:
   app dir: C:\Users\username\AppData\Local\Continuum\miniconda3\share\jupyter\lab
        @jupyter-voila/jupyterlab-preview v0.1.3 enabled  ok
        @jupyter-widgets/jupyterlab-manager v1.0.3 enabled  ok
        @jupyterlab/celltags v0.2.0 enabled  ok
        @jupyterlab/git v0.8.2 enabled  ok
        @jupyterlab/toc v1.0.1 enabled  ok
        nbdime-jupyterlab v1.0.0 enabled  ok

Command Line Output

Here's the error:
[E 17:03:01.489 LabApp] C:\
    Traceback (most recent call last):
      File "C:\Users\username\AppData\Local\Continuum\miniconda3\lib\site-packages\nbdime\webapp\nb_server_extension.py", line 97, in get_git_notebooks
        git_root = find_repo_root(file_path)
      File "C:\Users\username\AppData\Local\Continuum\miniconda3\lib\site-packages\nbdime\gitfiles.py", line 52, in find_repo_root
        repo = get_repo(path)[0]
      File "C:\Users\username\AppData\Local\Continuum\miniconda3\lib\site-packages\nbdime\gitfiles.py", line 41, in get_repo
        repo = Repo(path)
      File "C:\Users\username\AppData\Local\Continuum\miniconda3\lib\site-packages\git\repo\base.py", line 184, in __init__
        raise InvalidGitRepositoryError(epath)
    git.exc.InvalidGitRepositoryError: C:\
[W 17:03:01.498 LabApp] 422 POST /nbdime/api/gitdiff?1575410581479 (::1): Invalid notebook: C:\Users\username\Code\AOE\Users\username\Code\AOE\EDE\notebooks\examples\voila.ipynb
[W 17:03:01.502 LabApp] 422 POST /nbdime/api/gitdiff?1575410581479 (::1) 15.94ms referer=http://localhost:8888/lab
[E 17:03:18.133 LabApp] C:\
    Traceback (most recent call last):
      File "C:\Users\username\AppData\Local\Continuum\miniconda3\lib\site-packages\nbdime\webapp\nb_server_extension.py", line 97, in get_git_notebooks
        git_root = find_repo_root(file_path)
      File "C:\Users\username\AppData\Local\Continuum\miniconda3\lib\site-packages\nbdime\gitfiles.py", line 52, in find_repo_root
        repo = get_repo(path)[0]
      File "C:\Users\username\AppData\Local\Continuum\miniconda3\lib\site-packages\nbdime\gitfiles.py", line 41, in get_repo
        repo = Repo(path)
      File "C:\Users\username\AppData\Local\Continuum\miniconda3\lib\site-packages\git\repo\base.py", line 184, in __init__
        raise InvalidGitRepositoryError(epath)
    git.exc.InvalidGitRepositoryError: C:\
[W 17:03:18.136 LabApp] 422 POST /nbdime/api/gitdiff?1575410598126 (::1): Invalid notebook: C:\Users\username\Code\AOE\Users\username\Code\AOE\EDE\notebooks\examples\voila.ipynb

Browser Output

Please see request params above, no console output was issued for this error

[fcollonval]

Thanks for reporting @jamesjnadeau

There is indeed a bug in the file path sent to nbdime. In #406, I did a correction. If you can could you try to install that branch to see if it fixes your problem?

[jamesjnadeau]

I'm logging what I did to try this out locally in the hopes this can be added to the docs later...

1. Clone the repo:

git clone git@github.com:fcollonval/jupyterlab-git.git
cd jupyterlab-git
git checkout plaintextdiff-cm

1. Install the package from the local branch

cd ..
pip install -e jupyterlab-git

1. Enable the extension & rebuild jupyter lab assets

jupyter serverextension enable --py jupyterlab_git
jupyter lab build

1. Launch Jupyter Lab and test

[jamesjnadeau]

@fcollonval I tried to use your branch for pull#406 (see above steps I took) the plaintextdiff-cm branch did not recognize a git branch for me.

I also tried fix-471-absolute-path-nbdime branch and had similar results.

Here's the error I see in jupyter labs output for the fix-471-absolute-path-nbdime:

[E 08:48:50.261 LabApp] Uncaught exception POST /git/all_history?1575467330254 (::1)
    HTTPServerRequest(protocol='http', host='localhost:8888', method='POST', uri='/git/all_history?1575467330254', version='HTTP/1.1', remote_ip='::1')
    Traceback (most recent call last):
      File "C:\Users\nadeauj\AppData\Roaming\Python\Python37\site-packages\tornado\web.py", line 1697, in _execute
        result = method(*self.path_args, **self.path_kwargs)
      File "c:\users\nadeauj\code\jupyterlab-git\jupyterlab_git\handlers.py", line 56, in post
        history_count = body["history_count"]
    KeyError: 'history_count'
[W 08:48:50.264 LabApp] Unhandled error
[E 08:48:50.265 LabApp] {
      "Host": "localhost:8888",
      "User-Agent": "Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:70.0) Gecko/20100101 Firefox/70.0",
      "Accept": "*/*",
      "Accept-Language": "en-US,en;q=0.5",
      "Accept-Encoding": "gzip, deflate",
      "Referer": "http://localhost:8888/lab",
      "Content-Type": "application/json",
      "Origin": "http://localhost:8888",
      "Content-Length": "19",
      "Connection": "keep-alive",
      "Pragma": "no-cache",
      "Cache-Control": "no-cache"
    }
[E 08:48:50.267 LabApp] 500 POST /git/all_history?1575467330254 (::1) 4.99ms referer=http://localhost:8888/lab

reverting back to the version installed from conda allows the panel to work as expected(except the diff functionality of course).

I'm happy to test this more if you like or provide further feedback. Feel free to tag me @fcollonval if you need a windows tester.

[fcollonval]

@jamesjnadeau Thx for trying out. Actually you missed one step that rebuild the frontend extension. This is the source of the error you reported.

The commands to install from git as dev are the following:

pip install -e .[test]
jupyter serverextension enable --py jupyterlab_git

# Build the labextension and dev-mode link it to jlab
jlpm
jlpm build
jupyter labextension link .

Note: if you could try it on the branch fix-471-absolute-path-nbdime to ensure it covers your bug, it would be great.

[jamesjnadeau]

@fcollonval Thanks for the commands I missed, I was able to load the fix-471-absolute-path-nbdime branch locally. I see the new styling in the git panel, looks great!

I am still unfortunately seeing the same error, where the file_path variable in the post to get the diff still contains ../ at the beginning. It appears the bug still exists.

Happy to help further. I have some free time later this week to dive into the code myself now that I have dev env set up.

[fcollonval]

Thanks @jamesjnadeau for trying it out and sorry it did not fix the error.

Feel free to look into it and propose a PR. The request is done there:

https://github.com/jupyterlab/jupyterlab-git/blob/master/src/components/diff/NbDiff.tsx#L182-L186

So yes, the problem seems related to the file path being already absolute and the repo path being not appropriate.

[fcollonval]

So yes, the problem seems related to the file path being already absolute and the repo path being not appropriate.

One thought, is your git root folder accessible from the Jupyterlab root folder?
Like:
C:\path\to\jupyterlab
C:\path\to\jupyterlab\path\git_repo

Or

C:\path\git_repo\jupyterlab
C:\path\git_repo

[jamesjnadeau]

The root path I open Jupyter in(akathe root of the jupyters file browser) is my git repo. To say another way, the top most folder I can access in Jupyter contains my repo's .git file.

I'm going to try specifying a folder up the hierarchy when launching Jupyter to see if that makes a difference.

[jamesjnadeau]

Moving where jupyter starts made no difference. I went up two directory levels and am still receiving the same error when loading a diff.

[jamesjnadeau]

Just to level set here, this repo is detected when opening jupyterlab-git. The diffs also work flawlessly in a docker linux container I have with pretty much the same packages installed. I believe this is something window's specific, yay M$.... :)

[jamesjnadeau]

I went down the rabit hole for a bit, and I belive I have found the issue, but I'll need to test fixing it, i'm logging my assumption here for reference.
the implementation here:

jupyterlab-git/jupyterlab_git/handlers.py

Line 494 in f838270

# Similar to https://github.com/jupyter/nbdime/blob/master/nbdime/webapp/nb_server_extension.py#L90-L91

doesn't exactly replicate what is going on in nbdime, there is a call to os.path.realpath after that might solve my issue, need to test.

[fcollonval]

I successfully reproduce the bug. The error was coming from

jupyterlab-git/src/model.ts

Line 627 in d6b27c6

PathExt.relative(this._serverRoot, this.pathRepository),

PathExt.relative was not able to compute the relative path when the path is using backslashes. The fix is to work with a posix format in the frontend.

[jamesjnadeau]

@fcollonval any update on this, happy to help if you can point me in a direction?

I tried using the upath package to fix my issue, please see #516, but I imagine you'll want to make changes else where to fix this problem.

I noticed that the git filters I set up(nbstripout) were also not working for on windows for this extension. I would see it returned a 'non zero' exit status. turning off these filters fixed the issue. These filters work in other git programs, so it's something to do with the way this extension is running things. Happy to start a new issue for it.

[telamonian]

@jamesjnadeau I just merged in #507. Can you please check if your issue is fixed in the latest master?

#507 does basically the same thing as your #516, but it fixes the issue on the Python side (where the offending path originates).

If #507 fixes the path issue but you still see the issue with the git filters, please open a new issue for that.

[jamesjnadeau]

@telamonian I just tried the latest master branch as of writing this. It took a bit of cache clearing, but it appears it works!!!!, unless you use git filters.

I was not able to make this work with any git filter such as nbsctripout or fastai-nbstripout.

Without any git filter, the extension works as expected.

Here's the error reported by jupyter lab when using nbstripout installed globally:

'''
[E 14:02:12.273 LabApp] Uncaught exception POST /nbdime/api/diff?1579028531431 (127.0.0.1)
HTTPServerRequest(protocol='http', host='localhost:8888', method='POST', uri='/nbdime/api/diff?1579028531431', version='HTTP/1.1', remote_ip='127.0.0.1')
Traceback (most recent call last):
File "C:\Users\nadeauj\AppData\Local\Continuum\miniconda3\lib\site-packages\tornado\web.py", line 1699, in _execute
result = await result
File "C:\Users\nadeauj\AppData\Local\Continuum\miniconda3\lib\site-packages\tornado\gen.py", line 209, in wrapper
yielded = next(result)
File "C:\Users\nadeauj\AppData\Local\Continuum\miniconda3\lib\site-packages\nbdime\webapp\nb_server_extension.py", line 171, in post
base_nb, remote_nb = self.get_git_notebooks(base[len('git:'):])
File "C:\Users\nadeauj\AppData\Local\Continuum\miniconda3\lib\site-packages\nbdime\webapp\nb_server_extension.py", line 105, in get_git_notebooks
for fbase, fremote in changed_notebooks(ref_base, ref_remote, file_path, git_root):
File "C:\Users\nadeauj\AppData\Local\Continuum\miniconda3\lib\site-packages\nbdime\gitfiles.py", line 182, in changed_notebooks
entry.b_path, entry.b_blob, ref_remote, repo_dir)
File "C:\Users\nadeauj\AppData\Local\Continuum\miniconda3\lib\site-packages\nbdime\gitfiles.py", line 118, in _get_diff_entry_stream
ret = apply_possible_filter(path)
File "C:\Users\nadeauj\AppData\Local\Continuum\miniconda3\lib\site-packages\nbdime\vcs\git\filter_integration.py", line 81, in apply_possible_filter
stderr=STDOUT, shell=True
File "C:\Users\nadeauj\AppData\Local\Continuum\miniconda3\lib\subprocess.py", line 395, in check_output
**kwargs).stdout
File "C:\Users\nadeauj\AppData\Local\Continuum\miniconda3\lib\subprocess.py", line 487, in run
output=stdout, stderr=stderr)
subprocess.CalledProcessError: Command '"C:/Users/nadeauj/AppData/Local/Continuum/miniconda3/python.exe" "C:/Users/nadeauj/AppData/Local/Continuum/miniconda3/lib/site-packages/nbstripout"' returned non-zero exit status 1.
[W 14:02:12.280 LabApp] Unhandled error
'''

uninstalling nbstripout makes diffing work as expected.

running git status or git diff from the command line does work.

I feel this might be related to nbdime, but I'm not sure, I'm happy to submit a new issue as well, but right now this could be related?

Thanks so much for your effort here so far! I really appreciate it.

[jamesjnadeau]

I think it might be related to this issue reported in nbdime:
jupyter/nbdime#415 (comment)

you might need to do something similar when calling the subprocess for diffing?

[fcollonval]

@jamesjnadeau could you obtain the path use by nbdime in the filter call: https://github.com/jupyter/nbdime/blob/6259cd0936eecae3e0fbd4301ecc55f4a5e19bc2/nbdime/vcs/git/filter_integration.py#L77-L82

[jamesjnadeau]

here's where my local copy of nbdime is in my path:

(base) PS > where.exe nbdime.exe
C:\Users\username\AppData\Local\Continuum\miniconda3\Scripts\nbdime.exe

I'm not clear on what you mean by

obtain the path use by nbdime in the filter call

Based on my assumption that you want to know the contents of the 'path' variable in that function call, I installed nbdime locally and added the following python code above that line:

print('path = ' + str(path))

When run from jupyter, this errors as before, and I see the following returned by my print statement:

path = notebooks/path/to/notebook.ipynb

Hope this helps, I'm set up to help more if you need it.

Thanks again, I really appreciate you spending the time to figure this out for windows users. This extension is awesome, and makes jupyter lab a true IDE.

[fcollonval]

@jamesjnadeau I looked at this a bit more. This is a bug with nbstripout (issue reported kynan/nbstripout#115).

The filter command uses simple quote around the python executable path when it should be using double quote. The Windows console does not understand the single quote as string delimiter.

I was able to reproduce the error on my computer. But from your error message, it looks you are not using the latest version of nbstripout.

You got:

subprocess.CalledProcessError: Command '"C:/Users/nadeauj/AppData/Local/Continuum/miniconda3/python.exe" "C:/Users/nadeauj/AppData/Local/Continuum/miniconda3/lib/site-packages/nbstripout"' returned non-zero exit status 1.

I got with nbstripout 0.3.7

subprocess.CalledProcessError: Command ''C:/.../Anaconda3/envs/jlab/python.exe' -m nbstripout' returned non-zero exit status 1.

Could you confirm that?

[jamesjnadeau]

@fcollonval Thanks for looking into this more!
You are correct, I had nbstripout at version 0.3.6., and I'm producing a similar error to what you were seeing

after updating to 0.3.7

conda install -c conda-forge nbstripout=0.3.7

I'm getting the following error in Jupyter lab when diffing a file:

 subprocess.CalledProcessError: Command '"C:/Users/nadeauj/AppData/Local/Continuum/miniconda3/python.exe" "C:/Users/nadeauj/AppData/Local/Continuum/miniconda3/lib/site-packages/nbstripout"' returned non-zero exit status 1.
[W 09:16:30.957 LabApp] 500 POST /nbdime/api/gitdiff?1580134590038 (::1): Error while attempting to diff documents

I believe this confirms your assumption. Thanks again, and good catch on the quoting error!

[fcollonval]

@jamesjnadeau Could we close this issue as the remaining error is not related to this package?