Skip to content
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

RDoc doesn't have complete files when being installed from git/github source #1107

Open
st0012 opened this issue Apr 29, 2024 · 6 comments
Open

Comments

@st0012
Copy link
Member

st0012 commented Apr 29, 2024

Context

  • RDoc uses parser generator kpeg for its markdown parser, which reads lib/markdown.kpeg to output lib/markdown.rb.
  • RDoc doesn't commit the generated .rb files, but relies on its generate rake task to generate them.
  • generate is a prerequisite of the build task, which is part of the release task. So files like lib/markdown.rb will be generated and packaged as part of the gem.

However, if users install RDoc from a git/github source, those generated files will be missing, and thus cause errors.

Reproduction Steps

  1. Run the command below
$ bundle exec ruby -e 'require "rdoc"; require "rdoc/markdown"; puts RDoc::Markdown'
RDoc::Markdown
  1. Add gem "rdoc", github: "ruby/rdoc" to the Gemfile
  2. Run bundle install
  3. Run the same command again, which should now show an error like:
bundle exec ruby -e 'require "rdoc"; require "rdoc/markdown"; puts RDoc::Markdown'
/opt/rubies/3.3.0/lib/ruby/3.3.0/rdoc/markdown.rb:182:in `require': cannot load such file -- /Users/hung-wulo/.gem/ruby/3.3.0/bundler/gems/rdoc-8a68a016b25a/lib/rdoc/markdown (LoadError)
        from /opt/rubies/3.3.0/lib/ruby/3.3.0/rdoc/markdown.rb:182:in `<top (required)>'
        from -e:1:in `require'
        from -e:1:in `<main>'

Possible solutions

  • Commit the generated files into the git repository too, and have a way to automatically update them when their grammar files changes.
  • Utilize extconf.rb to run rake build, which will be executed every time the gem is installed (PoC).
  • Rely on other markdown parsers that don't require parser generator?
@colby-swandale
Copy link
Member

Not a maintainer, but wanted to provide my 2c

Commit the generated files into the git repository too, and have a way to automatically update them when their grammar files changes.

This is the easiest and fastest way to resolve this issue. I don't see any downside of doing this? Long term, the Markdown parser should be reviewed to see if the extra steps can be simplified/reduced into something not requiring compilation.

@zenspider
Copy link
Member

zenspider commented May 6, 2024

if users install RDoc from a git/github source

rake install:local generates the files in question before installing... so the problem is with what comes from bundle install, no?

ETA: I don't think this is a valid issue as it stands. I think this is a problem with bundler and the way people expect git bundles to work. Generated files should not be checked in w/o good reason, and I don't see a good reason here.

@st0012
Copy link
Member Author

st0012 commented May 6, 2024

so the problem is with what comes from bundle install, no?

While I agree it's not a "RDoc issue", I'm not sure if bundler should ever run a gem's build scripts locally. In RDoc's case, it means that bundler would also need to install at least kpeg, a development dependency of RDoc, on the user's machine. While that's the workaround I'm using, I don't think it should be a universal practice used by bundler.

(Of course I don't have much context around bundler, it's just my 2 cents)

Generated files should not be checked in w/o good reason

I think making it possible for the community to experiment with their RDoc patch or doing ad-hoc fixes when needed is a pretty good reason?

For example, my team is working on a new theme generator for RDoc called snapper (branch), which we're dogfooding in projects like Ruby LSP's docs. Without a workaround on this issue, it's impossible to do so. Currently we baked the extconf.rb workaround to make things work.

And if the community can't even properly dogfood the fixes/features they want to propose, it simply hinders the project's development. Given that the markdown parser of RDoc is not frequently touched, I think committing the generated files is a good tradeoff for enabling better a development flow for the project.

FWIW, though not for the exact same reason, debug also generates its readme dynamically and commit both the template and the generated result, with a simple task run on CI to make sure they're synced. It's been developed like this for over 2 years and works well IMO.

@rafaelfranca
Copy link

Just as another data point. I can't release the Rails docs with a fix from rdoc because that fix wasn't released yet, and I can't just point to the git repository. Either I have to fork rdoc to add a few generated files that always provide the same result, not matter which SO I generate them, or I need to wait a release of rdoc.

I can see the argument to not add the generated files to the repository, but I don't think this issue should be fixed in bundler because gem build inside this repository also builds an invalid gem. No tool will run or even know a rake install:local task exists.

If putting the generated file in the repository isn't a solution, maybe you should consider using extconf.rb to install the dependencies and generate the files that are missing.

@kddnewton
Copy link

For what it's worth, we ran into this same issue with prism. I didn't want to check in the generated files either, so I ended up doing the second approach here https://github.com/ruby/prism/blob/428d64bed407d34350c80ed83b41a2d59b8a1163/ext/prism/extconf.rb#L37. It's relatively simple all things considered.

@hsbt
Copy link
Member

hsbt commented May 23, 2024

@st0012 and all, Thanks for your consideration this.

I understood this issue, but I'm not sure what's best solution about this. If we put generated files to this repository, maintainers always check that don't update only generated files each pull-request.

@rafaelfranca

I need to wait a release of rdoc.

I just released rdoc-6.7.0 at https://github.com/ruby/rdoc/releases/tag/v6.7.0. You can use that instead of pointing rails/rdoc or ruby/rdoc at Gemfile of rails/rails.

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

No branches or pull requests

6 participants