Added tests for #556.

[dzhuang]

Testing #556 as mentioned in #767. The tests works without problems. The merge of this PR requires https://github.com/dzhuang/relate-sample/commit/f43b734c85de57fbab262ba5a19d0fe9d8733a4d being merged into sample repo.

P.S., We should mention in docs that path split should always use /.

@inducer

[inducer]

Thanks for doing this! I really don't understand why this works though. I tried whether dulwich does path parsing:

from dulwich.repo import Repo
r = Repo(".")
c = r[b"HEAD"]
t = r[c.tree]
f = t[b"pytools/__init__.py"]
print(f)

(run in a checkout of https://github.com/inducer/pytools/, because why not) It does not.

So if this code:

https://github.com/inducer/relate/blob/f4e8dee04dd8ac67d724412fe86957404ccb98cc/course/content.py#L651-L653

splits the path by os.sep (which i assume is \ in Windows, right?), how does it feed the right file names to dulwich? (I don't have a Windows system to check, otherwise I would)

[dzhuang]

OIC. I finally remembered the reason for proposing #556.

The .attribute file won't be parsed correctly in Windows. e.g., the file images/.attributes.yml will be images\.attributes.yml in Windows.

Now I can see this line (os.path.join)lead to that result: https://github.com/inducer/relate/blob/f4e8dee04dd8ac67d724412fe86957404ccb98cc/course/content.py#L803

~~I didn't realized that when I try to fix this issue, and in #556 the workround went to a wrong direction. Although it actually fixed it, I must admit it's bad and sorry for causing your confusion.~~

~~So the better workround is to use "/".join([os.path.dirname(path), ATTRIBUTES_FILENAME]), am I right?~~

See something interesting in #784

[dzhuang]

FYI, os.path.normpath

>>> os.path.normpath("repo/root/sub")
'repo\\root\\sub'
>>> os.path.normpath(r"repo/root/sub\.attribute") 
'repo\\root\\sub\\.attribute'

normpath will replace all \ and / to os.sep, and then we can use os.sep to split the path to elements.

[inducer]

I propose that we standardize on / as a path separator in course content. I'm not a fan of allowing both (as we would if we used normpath). I agree that using os.path.join to construct repo paths (in the spot you found, but there are a few more) is incorrect, we should fix those instances.

How do you feel about this direction?

[dzhuang]

I agree. But my first attempt failed (see #784). Maybe there're more os.path.join need to be changed as you mentioned.