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 site root #1626

Open
wants to merge 3 commits into
base: next
Choose a base branch
from
Open

Windows site root #1626

wants to merge 3 commits into from

Conversation

warsus
Copy link

@warsus warsus commented Sep 22, 2021

#1361

Next attempt 😄
Dont canonicalize paths because windows "\\?" prefix seems to break globbing.
If we have a relative path we turn it into an absolute one using the current_dir.
Maybe those are just workarounds for problems further down. In that case the make_abs_path calls could be removed later.

@ralfbiedert
Copy link

ralfbiedert commented Dec 10, 2021

I just tested this on Windows:

PR as-is

  • zola --root ..\..\cheats.rs build ok
  • zola --root ..\..\cheats.rs serve ok
  • zola --root d:\development\source\cheats.rs serve ok
  • cargo test ok

Ok here means it runs on Windows without visible errors and fixes #1692.

PR rebased on next

  • zola --root ..\..\cheats.rs build issue
  • zola --root ..\..\cheats.rs serve issue
  • zola --root d:\development\source\cheats.rs serve issue
  • cargo test ok

Rebasing this PR on top of 3155662 also seems to fix #1692 but does not fully expand shortcodes / markdown, producing HTML output like this:

<strong>The Book</strong> [<sup class="entry">BK</sup>](https://doc.rust-lang.org/book/),

s1

@Keats
Copy link
Collaborator

Keats commented Dec 10, 2021

Does the last issue happen on the next branch by itself?

@ralfbiedert
Copy link

Does the last issue happen on the next branch by itself?

Whops. I originally didn't bother testing since without this patch I got the error. But now that you mention it, this does happen on vanilla next 3155662. I thought I looked at the site after the byte panic patch, but apparently I haven't.

@Keats
Copy link
Collaborator

Keats commented Dec 10, 2021

Looking at the output, the shortcode just needs to be a .md instead of .html?

@ralfbiedert
Copy link

Ah, thanks. I wasn't aware that changed. Renaming my shortcodes to .md fixes it on next and this PR.

@Keats
Copy link
Collaborator

Keats commented Jan 26, 2022

Has anyone got an idea on how to test that with a unit test?

@Keats
Copy link
Collaborator

Keats commented Jun 13, 2022

@ralfbiedert do you know if the issue still happens on the next branch?

@ralfbiedert
Copy link

ralfbiedert commented Jun 14, 2022

Hm, for whatever reason I can't pull / switch to next anymore, I keep getting from the sublime-jinja2 submodule:

error: invalid path 'Preferences/Symbol List: Blocks.tmPreferences'
fatal: Could not reset index file to revision 'HEAD'.

Tried in my old clone and a fresh checkout. Nm, when I actually checkout with --branch instead of switching it works.

@ralfbiedert
Copy link

Test results: Using the latest next (5873e03)

  • zola --root ..\..\cheats.rs build issue
  • zola --root ..\..\cheats.rs serve issue
  • zola --root d:\development\source\cheats.rs serve issue
  • cargo test ok

The issue reported this time is a console error:

Building site...
Failed to build the site
Error: Failed to render content of //?/D:/Development/Source/cheats.rs/content/_index.md
Reason: Failed to render date shortcode
Reason: Tried to render `shortcodes/date.html` but the template wasn't found

That said, the reverse test (from cheats.rs directory) unfortunately fails too:

  • ..\_thirdparty\zola\target\release\zola.exe build issue
Building site...
Error: Failed to build the site
Error: Found path collisions:
- `/` from files ["\\\\?\\D:\\Development\\Source\\cheats.rs\\content\\_index.md", "//?/D:/Development/Source/cheats.rs/content/_index.md"]

Running that reverse test with 0.14.1 works just fine:

Building site...
Checking all internal links with anchors.
> Successfully checked 0 internal link(s) with anchors.
-> Creating 1 pages (0 orphan), 0 sections, and processing 0 images
Done in 208ms.

@Keats
Copy link
Collaborator

Keats commented Jun 14, 2022

Ok, I'll try to put this PR in then

@Keats
Copy link
Collaborator

Keats commented Jun 14, 2022

Ok I've fixed the conflicts on this branch, can you give it a try when you have time? I've tried it on mac but it was already working there :/

@ralfbiedert
Copy link

Sure. Note that right now the merged branch (968ba70) doesn't compile anymore (a use std::env; is missing in main.rs).

With that I still get the same error though:

❯ zola --root ..\..\cheats.rs build
Building site...
Failed to build the site
Error: Failed to render content of //?/D:/Development/Source/cheats.rs/content/_index.md
Reason: Failed to render date shortcode
Reason: Tried to render `shortcodes/date.html` but the template wasn't found

@Keats
Copy link
Collaborator

Keats commented Jun 15, 2022

Argh :(

@magikstm
Copy link
Contributor

magikstm commented Nov 5, 2022

Will this PR possibly be in the next version (0.17.0)?

@Keats
Copy link
Collaborator

Keats commented Nov 5, 2022

It needs to ideally have tests

@magikstm
Copy link
Contributor

magikstm commented Nov 5, 2022

I'm not really familiar with Rust at the moment.

But, why isn't "get_config_file_path" split into two functions. such as:
get_root_dir
get_config_file

I think it would be easier to create tests for both of them separately. They seem pretty critical to the rest of Zola's process.

@Keats
Copy link
Collaborator

Keats commented Jan 9, 2023

Can someone on Windows revive this with some tests?

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.

4 participants