jekyll / jekyll-sitemap

Jekyll plugin to silently generate a sitemaps.org compliant sitemap for your Jekyll site
http://rubygems.org/gems/jekyll-sitemap
MIT License
949 stars 134 forks source link

File conflict for page_without_a_file.rb with Jekyll #241

Closed dleidert closed 5 years ago

dleidert commented 5 years ago

It seems, jekyll also ships the file lib/jekyll/page_without_a_file.rb, which creates a file conflict. Can this file be removed in the next release of the jekyll-sitemap plugin?

ashmaroli commented 5 years ago

@dleidert More information would help. What exactly is the error you're getting? Also, please post the backtrace from running bundle exec jekyll build --trace

dleidert commented 5 years ago

@ashmaroli Both jekyll-sitemap and jekyll provide the same class file. That is a classic file conflict, which leads to two packages, provioding the same file, overwriting each others file again and again in the worst case. This gets really worse, if one of these files begins to differ, because it is developed further. This shouldn't happen. You were the person, who added this file nearly a year ago to the jekyll source. So you are probably the right person to address to fix this issue.

jekyll: https://github.com/jekyll/jekyll/blob/master/lib/jekyll/page_without_a_file.rb https://github.com/jekyll/jekyll/commits/master/lib/jekyll/page_without_a_file.rb

jekyll-sitemap: https://github.com/jekyll/jekyll-sitemap/blob/master/lib/jekyll/page_without_a_file.rb

ashmaroli commented 5 years ago

That is a classic file conflict, which leads to two packages, provioding the same file, overwriting each others file again and again in the worst case. This gets really worse, if one of these files begins to differ, because it is developed further.

@dleidert I really can't help unless you tell me what the exact issue is. Are you developing a plugin and facing issues with missing methods or is something not working as expected in jekyll-sitemap or are you simply stating the fact that there are files with same paths in different projects?

dleidert commented 5 years ago

The same class Jekyll::PageWithoutAFile cannot be provided by two packages, that can be installed together. You maybe don't notice it when using gems or bundler, because they tend to install every gem into its own directory. Further both currently provide the same content. But if you install the gem contents into a vendor path, like Linux distributions use to do, the file conflict becomes obvious. For example: In Debian both jekyll and jekyll-sitemap would try to install the file

/usr/lib/ruby/vendor_ruby/jekyll/page_without_a_file.rb

and create a conflict. I'm also beginning to wonder, why you think, it is normal, that two ruby packages provide the same class here?! That is an issue in every language I know. If you try to load that class: which one will be loaded? JFTR: ATM both differ on my system:

diff -puN ../ruby-jekyll-sitemap/lib/jekyll/page_without_a_file.rb /tmp/jekyll-3.8.3+dfsg/lib/jekyll/page_without_a_file.rb 
--- ../ruby-jekyll-sitemap/lib/jekyll/page_without_a_file.rb    2019-03-07 05:58:47.383795340 +0100
+++ /tmp/jekyll-3.8.3+dfsg/lib/jekyll/page_without_a_file.rb    2018-06-05 15:23:52.000000000 +0200
@@ -1,9 +1,18 @@
 # frozen_string_literal: true

 module Jekyll
+  # A Jekyll::Page subclass to handle processing files without reading it to
+  # determine the page-data and page-content based on Front Matter delimiters.
+  #
+  # The class instance is basically just a bare-bones entity with just
+  # attributes "dir", "name", "path", "url" defined on it.
   class PageWithoutAFile < Page
     def read_yaml(*)
       @data ||= {}
     end
+
+    def inspect
+      "#<Jekyll:PageWithoutAFile @name=#{name.inspect}>"
+    end
   end
 end
ashmaroli commented 5 years ago

I'm also beginning to wonder, why you think, it is normal

For the record, I do not consider this to be normal. The reason I proposed to include this class in Jekyll Core is because multiple in-house plugins "define" this same class identically. At that time, the plugins' maintainers did not wish to drop support for older versions of Jekyll mainly because in Ruby, the file path conflict is gracefully handled.

In Jekyll Core, this class is lazily loaded:

autoload :PageWithoutAFile, "jekyll/page_without_a_file"

In other words, the first time, Jekyll::PageWithoutAFile is referenced during runtime, the path"jekyll/page_without_a_file" is looked up across the $LOAD_PATH global array and loaded once if found. Jekyll Core take precedence because it will be near the beginning in the load_path array.

The same class in jekyll-sitemap (and other plugins with same internal path) are rendered redundant or in other words dead code. If the paths are different, then the later loaded code takes precedence and overrides the previous class definition entirely.

ashmaroli commented 5 years ago

pinging @jekyll/build and @jekyll/ecosystem for additional inputs..

miklb commented 5 years ago

@ashmaroli that sounds like a sensible solution for run time but I could see how trying to write to same location could present a problem. If core already has this present, and it’s dead code in the plugin, I don’t see why not just remove it out of caution.

jekyllbot commented 5 years ago

This issue has been automatically marked as stale because it has not been commented on for at least two months.

The resources of the Jekyll team are limited, and so we are asking for your help.

If this is a bug and you can still reproduce this error on the master branch, please reply with all of the information you have about it in order to keep the issue open.

If this is a feature request, please consider whether it can be accomplished in another way. If it cannot, please elaborate on why it is core to this project and why you feel more than 80% of users would find this beneficial.

This issue will automatically be closed in two months if no further activity occurs. Thank you for all your contributions.