Zola: Auto reload not working on windows

[kwellman]

Bug Report

Environment

Zola version: 0.6.0 (windows/powershell)

Expected Behavior

Running zola serve and then making a change to a file will cause the server to reload the files without any problems on macos. Making the same change when running the same site on windows will cause the server to exit with an error.

Current Behavior

This exception is thrown:

Change detected @ 2019-03-29 11:07:54
thread 'main' panicked at 'internal error: entered unreachable code: Got a change in an unexpected path: C:\Users\Kenji\Projects\TestSite\content/_index.md', src\cmd\serve.rs:489:9
stack backtrace:
   0: <unknown>
   1: <unknown>
   2: <unknown>
   3: <unknown>
   4: <unknown>
   5: <unknown>
   6: <unknown>
   7: <unknown>
   8: <unknown>
   9: onig_unicode_define_user_property
  10: BaseThreadInitThunk
  11: RtlUserThreadStart

(The mix of backslashes and forward slashes in the "unexpected path" looks really fishy to me)

Step to reproduce

Run zola serve on windows in the powershell. If you make any change to a file an exception will be thrown.

[Keats]

C:\Users\Kenji\Projects\TestSite\content/_index.md

Dammit \ /

If anyone on Windows want to debug/fix that, it should be fairly easy, seems that the issue is really in cmd/serve.rs where the match is expecting paths with /. Not entirely sure how that issue slipped by though.

[senden9]

Maybe additionally in the replace of https://github.com/getzola/zola/blob/b443be2365ecc012192fb4ec1e94ee76bf02389d/components/site/src/lib.rs#L75-L76

[Keats]

This line should be fine, it hasn't changed in 5 months.

Is the issue only appearing in serve and not in build? If someone on Windows can figure it out, I'll push a 0.6.1 asap

[ericharding]

I printed out the value of pwd here and it's not just a case of the wrong slashes.

thread 'main' panicked at 'internal error: entered unreachable code: Got a change in an unexpected path: d:\hg\blog\content/blog\xinetd\index.md pwd=\?\D:\hg\blog', src\cmdserve.rs:494:9

It seems like bp.canonicalize() is the culprit. If I remove that from serve.rs:408 it works again.

This was added in https://github.com/getzola/zola/commit/1815155c1d48460348be165a4044cbd936f529db AFAICT this was just added defensively rather than to fix a specific issue. I _think_ it might work to just remove the canonicalize call since Windows is pretty forgiving on path slashes these days. What do you think?

https://doc.rust-lang.org/std/fs/fn.canonicalize.html
"On Windows, this converts the path to use extended length path syntax..."

[ericharding]

Oh wait, better idea. We can just canonicalize path in detect_change_kind before we try to strip the prefix. I'll make a PR.

[Keats]

"On Windows, this converts the path to use extended length path syntax..."

I did miss that part in the docs. I think a good solution would have:

1. a test for the current issue if possible so it won't happen again
2. dropping the canonicalize and see if all tests are passing again (cc @blakesmith), I'll be able to test on Linux/Mac on my end

[ericharding]

If I remove the canonicalize() the tests do pass on both Windows/Linux (WSL). I don't know the code base (or really rust) well enough to know if this might break anything else.

[Keats]

I'll have a look at adding some tests for the base-path flag so we don't break anything, I don't think there are any right now.
The good news is that serve works with just that change so it is much simpler than what I thought it was.

[blakesmith]

Thanks for the ping on this one. I can review more soon: from memory, I remember adding the canonicalize call to handle relative / absolute paths consistently. I'll report back to this thread.

[ericharding]

It can definitely be made to work with canonicalize() but we'd have to re-think how the tests work since it needs the file to exist. We could handle the error but then it would always fail for tests and always pass when running which doesn't make for a great test.

[Keats]

Looks like the base-path command line option does not work with shortcodes currently as well. I think I'll revert that commit for now and add that feature back later with tests, unless it can be fixed/tested quickly

[blakesmith]

@Keats Can you give an example of a shortcodes case that fails?

[Keats]

It looks like it is very sensitive to / in the path, from the root of the Zola repo:

$ zola serve -b docs
$ zola serve -b docs/

The first line works and the second doesn't

[Keats]

I'll revert & publish this Saturday unless it is fixed, the Windows version has already been broken for too long

[Keats]

If anyone on Windows can try the next branch to confirm it's working now!

[Keats]

ping @kwellman @ericharding

[ericharding]

Update notifications seem to be working in next. It looks like the -b option was removed in next so I didn't test that.

[ericharding]

To be more clear. Next branch is working fine for me.

[Keats]

0.7.0 is building right now, will be fixed once it's done

[ericharding]

Thanks Keats. Appreciate all your work on Zola.