8000 Allow static files to be symlinked in unsafe mode or non-prod environments by surrim · Pull Request #4640 · jekyll/jekyll · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Allow static files to be symlinked in unsafe mode or non-prod environments #4640 8000

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Mar 25, 2016
Merged

Allow static files to be symlinked in unsafe mode or non-prod environments #4640

merged 4 commits into from
Mar 25, 2016

Conversation

surrim
Copy link
@surrim surrim commented Mar 6, 2016

This patch enables symlinks for static files. Now you can do something like this:

news/
    new-pc.jpg -> ../images/DSC0001.jpg (21 bytes)
    _posts/
        2016-03-06-new-pc.md
images/
    DSC0001.jpg (1,2 MiB)

The generated page could be something like that:

_site/
    news/
        2016-03-06-new-pc.html
        new-pc.jpg -> ../images/DSC0001.jpg (21 bytes, 1,2 MiB with jekyll 3.1.2)
    images/
        DSC0001.jpg (1,2 MiB)
  • no need to grep for invalid URLs (e.g. DSC0001.jpg becomes dsc0001.jpg), written things can stay untouched when filenames or folders change
  • detecting broken symlinks is easy (file explorer, ls -l, etc.)
  • nice hyperlinks for visitors (/news/new-pc.jpg) without grown webspace requirements and reduced jekyll build time

parkr
parkr reviewed Mar 9, 2016
View reviewed changes
lib/jekyll/static_file.rb
@@ -80,7 +80,7 @@ def write(dest)

FileUtils.mkdir_p(File.dirname(dest_path))
FileUtils.rm(dest_path) if File.exist?(dest_path)
FileUtils.cp(path, dest_path)
FileUtils.copy_entry(path, dest_path)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please wrap this around a @site.safe call? We cannot use copy_entry in safe mode.

if @site.safe || Jekyll.env.start_with?("prod")
  FileUtils.cp(path, dest_path)
else
  FileUtils.copy_entry(path, dest_path)
end

The Jekyll.env.start_with? "prod" ensures production builds (where the destination and source may not be on the same host) work as expected.

@parkr parkr changed the title enable symlinks for static files Allow static files to be symlinked in unsafe mode or non-prod environments Mar 9, 2016
@parkr parkr added enhancement pending-feedback We are waiting for more info. and removed undetermined labels Mar 9, 2016
@surrim
Copy link
Author
surrim commented Mar 11, 2016

I just updated the code and tested it, works fine here. Thanks for your assistance.

parkr
parkr reviewed Mar 24, 2016
View reviewed changes
lib/jekyll/static_file.rb
@@ -80,7 +80,11 @@ def write(dest)

FileUtils.mkdir_p(File.dirname(dest_path))
FileUtils.rm(dest_path) if File.exist?(dest_path)
FileUtils.cp(path, dest_path)
if @site.safe
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am happy to accept this if you add Jekyll.env == "production" here.

Production builds should produce an artifact (the compiled site) that is standalone and does not have any external dependencies on other files. A symlink cannot be guaranteed to validate that requirement.

@surrim
Copy link
Author
surrim commented Mar 25, 2016

I added the condition again.
Could you please add some more documentation about safe mode? Maybe updating the configuration page with a GitHub hint would be enough.

@surrim surrim closed this Mar 25, 2016
@surrim surrim reopened this Mar 25, 2016
@parkr
Copy link
Member
parkr commented Mar 25, 2016

@surrim Sounds good!

@parkr
Copy link
Member
parkr commented Mar 25, 2016

@jekyllbot: merge +minor

@jekyllbot jekyllbot merged commit 1ab0ed3 into jekyll:master Mar 25, 2016
jekyllbot added a commit that referenced this pull request Mar 25, 2016
@jekyllbot
Update history to reflect merge of #4640 [ci skip]
f16c5d0
parkr added a commit that referenced this pull request Mar 26, 2016
@parkr
Merge remote-tracking branch 'origin/master' into themes
a30068b 
* origin/master: (65 commits)
  Update history to reflect merge of #4703 [ci skip]
  Update history to reflect merge of #4712 [ci skip]
  Highlight the test code
  Update history to reflect merge of #4640 [ci skip]
  readded "env=prod"-condition
  Update history to reflect merge of #3849 [ci skip]
  Update history to reflect merge of #4624 [ci skip]
  Update history to reflect merge of #4704 [ci skip]
  Update history to reflect merge of #4706 [ci skip]
  Checks for link file extension in tests
  Updating assets documentation
  Fix test teardown for cleaner.
  Update history to reflect merge of #4542 [ci skip]
  Add explanation of site variables in the example _config.yml
  Use double quotes in the gemfile
  Add test for creation of Gemfile by 'jekyll new'
  Add comment about github-pages
  Update history to reflect merge of #4533 [ci skip]
  Ensure Rouge closes its div/figure properly after highlighting ends.
  Add Site#config= which can be used to set the config
  ...
@Crunch09 Crunch09 mentioned this pull request Sep 6, 2016
Merged
@stomar stomar mentioned this pull request Sep 8, 2016
Closed
17 tasks
@DirtyF DirtyF mentioned this pull request May 25, 2018
Open
6 tasks
@jekyll jekyll locked and limited conversation to collaborators Jul 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
frozen-due-to-age pending-feedback We are waiting for more info.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@surrim @parkr @jekyllbot
0