d4l-data4life / kirby3-static-site-generator

Static site generator plugin for Kirby 3+. With this plugin you can create a directory with assets, media and static html files generated from your pages. The result is an even faster site with less potential vulnerabilities.
MIT License
139 stars 7 forks source link

Using option 'preserve' => ['filename'] doesn't preserve 'filename' #47

Closed bytesource closed 2 years ago

bytesource commented 2 years ago

I'm hosting my Kirby static site on Cloudflare Pages. To set up 301 redirects, I need to add a file _redirects (no file extension) to the root output folder.

config.localhost.php

return [
    'd4l' => [
          'static_site_generator' => [
           'endpoint' => 'update-static-site',
            // https://developers.cloudflare.com/pages/platform/redirects/
           'preserve' => ['_redirects', 'test', 'ext.txt'],
        ]
    ],
];

However, setting up the preserve option like this always removes the file when running the site generator:

By the way, I'm using a static-site-generator field. I also tried changing the name to test(in case the plugin doesn't pick up underscores) and ext.txt (in case it expects a file extension). But they all were removed from the output folder, too.

Expected Behavior

Files listed with preserve should be preserved in the static file folder.

Current Behavior

Possible Solution

Steps to Reproduce

Context

Kirby version: 3.6.1.1 PHP version: 8.0.13 Installation of kirby3-static-site-generator via Composer.

Configuration: Blueprint:

fields: 
    staticSiteGenerator:
        label: Generate a static version of the site

Configuration: See top of post

I looked through past issues (searching for 'preserve'), but couldn't find any related topics. Of course, it's possible I did some stupid mistake, but I'm not sure what that might be.

So, if you could take a look at this mystery, that would be great.

Cheers,

Stefan

jonathan-reisdorf commented 2 years ago

Hi Stefan :wave: Thanks for posting the issue. I'm using the preserve option on some sites as well and there it works, but I'm always directly invoking the class rather than using the field. However, to reproduce the issue I have now taken the starterkit and added the configuration you posted. Unfortunately I'm still not able to reproduce the issue, _redirects is preserved in the static folder (tested on Linux). I have pushed the starterkit along with the modifications I did to debug this issue to the following repo: https://github.com/jonathan-reisdorf/kirby-ssg-issue47 Here's the commit with the changes: https://github.com/jonathan-reisdorf/kirby-ssg-issue47/commit/874903d05945ded88ab0e70f428ed558361c306d Would be great if you could clone that repo (or the starterkit and then apply the changes) and let me know if you run into the same issue (both locally as well as on CloudFlare) with the codebase. If it works for you locally, but not on CloudFlare, that would indicate that something on that service is causing the issue. Thanks!

bytesource commented 2 years ago

Hi Jonathan,

Thanks for taking the time to set up the test environment!

I cloned the repo, added _redirects to the root folder and ran the plugin. Unfortunately, the file was gone afterwards.

Here are two screenshots of before and after running the plugin.

Before: Add file _redirects SSG Plugin - Issue 47 - add '_redirects' file

After: Missing file _redirects SSG Plugin - Issue 47 - '_redirects' file missing after running plugin

Not sure if this is important, but the site is installed on Windows 10 using Wampserver 3.2.6.

Is there any additional information I could provide you with?

Cheers,

Stefan

jonathan-reisdorf commented 2 years ago

Thank you for your response and the additional information. I can now reproduce the issue :raised_hands: Must be something with Windows resolving the paths differently. I'll debug now and then create a PR with the fix.

bytesource commented 2 years ago

Thank you so much!

I am not sure if it helps, here are a few observations I made:

Passing ['_redirects'] directly to the generate() function still removes _redirects.

 $list = $staticSiteGenerator->generate($outputFolder, $baseUrl, ['_redirects']);
 // $list = $staticSiteGenerator->generate($outputFolder, $baseUrl, $preserve); 

https://github.com/d4l-data4life/kirby3-static-site-generator/blob/master/index.php#L34

The preserve option is used in the cleanFolder() function. Looking at the code, it should work just fine:

public function clearFolder(string $folder, array $preserve = [])
  { 
    $folder = $this->_resolveRelativePath($folder);
    $items = $this->_getFileList($folder);
    return array_reduce($items, function ($totalResult, $item) use ($preserve) {
      $folderName = $this->_getFolderName($item);
      if (in_array($folderName, $preserve)) {
        return $totalResult;
      }

      if (strpos($folderName, '.') === 0) {
        return $totalResult;
      }

      $result = is_dir($item) === false ? F::remove($item) : Dir::remove($item);
      return $totalResult && $result;
    }, true);
  }

https://github.com/d4l-data4life/kirby3-static-site-generator/blob/master/class.php#L262

You say that it works on Linux, so an issue with the path or path separator indeed comes to mind first, e.g., if either $this->_getFileList($folder) or $this->_getFolderName($item) returned the wrong result on Windows, in_array($folderName, $preserve) couldn't find a match anymore.

jonathan-reisdorf commented 2 years ago

Thanks, yes, I found the issue now. Kirby's Dir::read method returns inconsistent directory separator characters on Windows, so the first segments are separated by \, but the last one is separated by /. I have already locally added a fix circumventing this and will push it in a few mins.

Bildschirmfoto von 2022-03-30 09 44 22

bytesource commented 2 years ago

I think that I found the cause of the issue. This is what I did:

1) Add a file named _redirects to the root of the output folder /static.

2) Add the following lines to clear Folder:

if (strpos($folderName, '_redirects')) {
        $msg = "folderName: " . $folderName . ", item: " . $item;
        print($msg);
}

This, of course, is not a best practice when it comes to debugging, but I'm not a developer and never got XDebug to work…

3) Run the plugin The above code crashes the program with a JSON could not be parsed error, but still produces the output from the print statement:

This is what the output looked like:

folderName: static/_redirects, item: C:\wamp64\www\kirby-ssg-issue47\static/_redirects

So, yes, this is probably a path issue.

bytesource commented 2 years ago

That was fast. Thank you so much!

bytesource commented 2 years ago

Thanks for resolving the issue!

The new code this array_map, right?

array_map(function($item) {
      return str_replace('/', DIRECTORY_SEPARATOR, $item);
    }, Dir::read($path, [], true));

Strangely, now I'm getting this error:

Hello! It seems the given output folder "C:\wamp64\www\kirby-ssg-issue47\static" already contains other files or folders. Please specify a path that does not exist yet, or is empty. If it absolutely has to be this path, create an empty .kirbystatic file and retry. WARNING: Any contents of the output folder not starting with "." are erased before generation! Information on preserving individual files and folders can be found in the Readme.

Going through the README, I couldn't find a way to turn the warning off. https://github.com/d4l-data4life/kirby3-static-site-generator#warnings

Do I have to empty the output folder manually before running the plugin? But then again, adding the _redirects file to the empty folder will trigger the warning anyway.

Any advice?

PS. If you have a few extra minutes, could you reopen or comment on this issue: https://github.com/d4l-data4life/kirby3-static-site-generator/issues/35#issuecomment-1068892207

To be fair, it's probably not an issue, but my use case of moving a file named 404.html to the root folder likely just doesn't fit.

jonathan-reisdorf commented 2 years ago

@bytesource actually, this LOC here should resolve the issue you are describing above: https://github.com/d4l-data4life/kirby3-static-site-generator/commit/2f8fd80bb3737ecc25b958a0bb824cc3ca917071#diff-824c5095fca730bbb42b15a306c41d2e90bc2385c277cfb91b5c790bb37a4013R349 There should be a .kirbystatic file in your static folder, which acts as a flag for the generator so it knows that this is a folder where it already generated files in the past, so it's safe to erase everything in there except for the preserved files. How did you update to the new version of the static site generator?

bytesource commented 2 years ago

If it absolutely has to be this path, create an empty .kirbystatic file and retry.

Sorry, I overlooked this sentence from the README. Adding a file named .kirbystatic to the root of the output folder got rid of the warning.

After updating using Composer, copied _redirects to the root folder, ran the plugin… and it worked! _redirects was indeed preserved and still present in the folder.

Thank you so much for your help!