Layout: set relative_path without using Pathname #5164

Merged
merged 1 commit into from Aug 2, 2016

Conversation

Projects
None yet
4 participants
@parkr
Member

parkr commented Jul 29, 2016

Presently, on a Windows machine, you get an ArgumentError on Windows:

     Generating...
C:/Ruby23-x64/lib/ruby/2.3.0/pathname.rb:520:in `relative_path_from':
    different prefix: "/" and "C:/Ruby23-x64/lib/ruby/gems/2.3.0/gems/minima-1.0.1" (ArgumentError)
    from C:/Ruby23-x64/lib/ruby/gems/2.3.0/gems/jekyll-3.2.0/lib/jekyll/layout.rb:61:in `relative_path'
    from C:/Ruby23-x64/lib/ruby/gems/2.3.0/gems/jekyll-3.2.0/lib/jekyll/renderer.rb:161:in `place_in_layouts'
    from C:/Ruby23-x64/lib/ruby/gems/2.3.0/gems/jekyll-3.2.0/lib/jekyll/renderer.rb:71:in `run'

This patch attempts to fix that. Creating a PR so I can use AppVeyor.

This doesn't affect filesystems which do not use drive names.

/cc @jekyll/windows

Layout: set relative_path without using Pathname
Presently, on a Windows machine, you get an ArgumentError on Windows:

     Generating...
C:/Ruby23-x64/lib/ruby/2.3.0/pathname.rb:520:in `relative_path_from':
    different prefix: "/" and "C:/Ruby23-x64/lib/ruby/gems/2.3.0/gems/minima-1.0.1" (ArgumentError)
    from C:/Ruby23-x64/lib/ruby/gems/2.3.0/gems/jekyll-3.2.0/lib/jekyll/layout.rb:61:in `relative_path'
    from C:/Ruby23-x64/lib/ruby/gems/2.3.0/gems/jekyll-3.2.0/lib/jekyll/renderer.rb:161:in `place_in_layouts'
    from C:/Ruby23-x64/lib/ruby/gems/2.3.0/gems/jekyll-3.2.0/lib/jekyll/renderer.rb:71:in `run'

This doesn't affect filesystems which do not use drive names.

@parkr parkr added this to the 3.2.1 milestone Jul 29, 2016

@parkr parkr added the fix label Jul 29, 2016

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Jul 29, 2016

Member

@XhmikosR Would you mind giving this SHA a try on your site? I believe this should do it. Down to 15 failures on AppVeyor.

Fixes #5146.

Member

parkr commented Jul 29, 2016

@XhmikosR Would you mind giving this SHA a try on your site? I believe this should do it. Down to 15 failures on AppVeyor.

Fixes #5146.

@qbikez

This comment has been minimized.

Show comment
Hide comment
@qbikez

qbikez Jul 30, 2016

This fixed #4677 for me (in a fresh jekyll instance).

qbikez commented Jul 30, 2016

This fixed #4677 for me (in a fresh jekyll instance).

@XhmikosR

This comment has been minimized.

Show comment
Hide comment
@XhmikosR

XhmikosR Jul 30, 2016

Contributor

@parkr: confirmed, it fixes the issue for me on Windows.

Contributor

XhmikosR commented Jul 30, 2016

@parkr: confirmed, it fixes the issue for me on Windows.

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Aug 1, 2016

Member

LGTM. @jekyll/core, would you mind giving this another LGTM?

Member

parkr commented Aug 1, 2016

LGTM. @jekyll/core, would you mind giving this another LGTM?

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Aug 2, 2016

Member

🆒

@jekyllbot: merge +bug

Member

parkr commented Aug 2, 2016

🆒

@jekyllbot: merge +bug

@jekyllbot jekyllbot merged commit 6e0119d into master Aug 2, 2016

1 of 3 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
Details
jekyll/lgtm Approved by @parkr. Requires 1 more LGTM.
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@jekyllbot jekyllbot added bug fix labels Aug 2, 2016

@jekyllbot jekyllbot deleted the fix-windows-error branch Aug 2, 2016

jekyllbot added a commit that referenced this pull request Aug 2, 2016

@XhmikosR

This comment has been minimized.

Show comment
Hide comment
@XhmikosR

XhmikosR Aug 2, 2016

Contributor

@parkr: really appreciate it. Do you think the remaining Windows failures could be fixed so that it's easier to spot any failures in the future?

Contributor

XhmikosR commented Aug 2, 2016

@parkr: really appreciate it. Do you think the remaining Windows failures could be fixed so that it's easier to spot any failures in the future?

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Aug 2, 2016

Member

@XhmikosR Yeah, I would like to do that. I'll put together an issue tomorrow (I'm in SF) and hopefully some of our amazing fellow contributors can lend a hand. 😄

Member

parkr commented Aug 2, 2016

@XhmikosR Yeah, I would like to do that. I'll put together an issue tomorrow (I'm in SF) and hopefully some of our amazing fellow contributors can lend a hand. 😄

@parkr parkr referenced this pull request Aug 2, 2016

Closed

Get Jekyll Tests Working on Windows #5179

0 of 9 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment