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

Raise when theme root directory is not available #6455

Merged
merged 1 commit into from Oct 28, 2017

Conversation

Projects
None yet
5 participants
@angelikatyborska
Contributor

angelikatyborska commented Oct 20, 2017

Fixes #5575 by raising RuntimeError with a meaningful message when the directory to which a theme was installed is removed:

$ bundle exec jekyll serve
Configuration file: /Users/angelika/Documents/jekyll-test/_config.yml
jekyll 3.6.0 | Error:  Path /Users/angelika/.rvm/gems/ruby-2.3.1/gems/minima-2.1.1 does not exist, is not accessible or includes a symbolic link loop

I concluded from the test suite that returning nil in this method is desired:

def realpath_for(folder)
File.realpath(Jekyll.sanitized_path(root, folder.to_s))
rescue Errno::ENOENT, Errno::EACCES, Errno::ELOOP
nil
end
because loading files from _includes, _layouts, _sass and assets is supposed to be skipped if their path is nil.

What is more, when the root directory exists (but has no contents), Jekyll does not raise, but provides warnings:

$ bundle exec jekyll serve
Configuration file: /Users/angelika/Documents/jekyll-test/_config.yml
            Source: /Users/angelika/Documents/jekyll-test
       Destination: /Users/angelika/Documents/jekyll-test/_site
 Incremental build: disabled. Enable with --incremental
      Generating... 
     Build Warning: Layout 'post' requested in _posts/2017-10-20-welcome-to-jekyll.markdown does not exist.
     Build Warning: Layout 'default' requested in 404.html does not exist.
     Build Warning: Layout 'page' requested in about.md does not exist.
     Build Warning: Layout 'home' requested in index.md does not exist.
                    done in 0.66 seconds.

Given all this, I only changed the behavior for getting the real path of the theme's root.

@pathawks

Wow! Nice work getting to the bottom of this. 🔎🎉👍🏼

@DirtyF DirtyF added fix bug labels Oct 20, 2017

@mattr-

This comment has been minimized.

Member

mattr- commented Oct 28, 2017

Thank you for fixing this! 🎊 🎉 🥇

@mattr-

This comment has been minimized.

Member

mattr- commented Oct 28, 2017

@jekyllbot: merge +bug

@jekyllbot jekyllbot merged commit 8dbe5de into jekyll:master Oct 28, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@jekyllbot jekyllbot added bug fix labels Oct 28, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment