Skip to content

Fix cop behavior in nested projects - #62

Open
mvz wants to merge 1 commit into
masterfrom
handle-nested-gem-projects
Open

mvz wants to merge 1 commit into
masterfrom
handle-nested-gem-projects

Conversation

@mvz

@mvz mvz commented Aug 29, 2026

Copy link
Copy Markdown
Collaborator

This considers the project root for a gem to be packaged to be determined by the presence of a Gemfile or gems.rb file.

It isolates the specs using RuboCop's shared 'isolated environment' context, triggered by the :isolated_environment annotation. This runs the specs in a temporary directory so we can set up the files the cop should look for to determine the (potentially nested) project root.

The RuboCop::ConfigFinder.project_root attribute turns out to be little used in RuboCop itself, to the point that it is not reset in the 'isolated environment' spec context. Therefore, we drop its use and simply rely on find_file_upwards to traverse the correct directory hierarchy.

This fixes #61.

This considers the project root for a gem to be packaged to be
determined by the presence of a Gemfile or gems.rb file.

It isolates the specs using RuboCop's shared 'isolated environment'
context, triggered by the :isolated_environment annotation. This runs
the specs in a temporary directory so we can set up the files the cop
should look for to determine the (potentially nested) project root.

The RuboCop::ConfigFinder.project_root attribute turns out to be little
used in RuboCop itself, to the point that it is not reset in the
'isolated environment' spec context. Therefore, we drop its use and
simply rely on find_file_upwards to traverse the correct directory
hierarchy.

This fixes #61.
@mvz
mvz force-pushed the handle-nested-gem-projects branch from cd3ed41 to adf5d0e Compare August 29, 2026 10:58
@mvz
mvz requested a review from utkarsh2102 August 29, 2026 10:58
@mvz mvz mentioned this pull request Aug 29, 2026
@pboling

pboling commented Aug 30, 2026

Copy link
Copy Markdown

@mvz I had AI analyze and compare my forked branch (from the closed PR) which respects nearest gemspec, and this one. Here's what it said:

Tested the upstream handle-nested-gem-projects branch:

- Its own suite passes: 55 examples, 0 failures.
- It works for normal StructuredMerge subgems because they have their own Gemfile.
- Your concern is correct: it defines package root solely by the nearest Gemfile or gems.rb, not a gemspec.

A real Kettle Jem nested fixture has example.gemspec but no Gemfile. On that fixture, upstream reports no RequireRelativeHardcodingLib offense; our fork correctly reports one by selecting the nearest gemspec as the package root.

The PR description explicitly says Gemfile/gems.rb is its definition of a packaged gem’s root; it does not give a semantic reason to avoid gemspecs. It appears to use RuboCop’s existing file-finder machinery, but loses gemspec-only nested projects and can fail if no ancestor Gemfile exists.

Do not switch from our fork (3539ba8). Its nearest-gemspec implementation is the correct behavior for our workspace.

I have a test fixture ensuring my dev harness works on a gem that has a gemspec but no Gemfile. That seems like a valid target for a rubocop project concerned with "packaging", since Ruby packages are most frequently rubygems, and gemspecs are part of the explicit profile of a rubygem.

Is the avoidance of detecting gemspecs intentional? And if so, why does that make sense (even for a monorepo with multiple gems and gemspecs - each gemspec represents a gem, which is a package)?

@mvz

mvz commented Aug 30, 2026

Copy link
Copy Markdown
Collaborator Author

@pboling AI;DR. I'm using Gemfile and gems.rb to match the existing RuboCop behavior of how it finds the project root. In practice, this should have the same result. Since your case is the only existing known case (there will surely be others!), my question remains: Does this give the expected result for your case?

@pboling

pboling commented Aug 30, 2026 •

Copy link
Copy Markdown

The code block was the AI part, but it seems you skipped what I wrote as well, so I will copy it here:

I have a test fixture ensuring my dev harness (about 70 gems and growing) works on a gem that has a gemspec but no Gemfile. That seems like a valid target for a rubocop project concerned with "packaging", since Ruby packages are most frequently rubygems, and gemspecs are part of the explicit profile of a rubygem, while Gemfile/gems.rb is not.

Is the avoidance of detecting gemspecs intentional? And if so, why does that make sense (even for a monorepo with multiple gems and gemspecs - each gemspec represents a gem, which is a package)?

Gemfile and gems.rb are package boundaries in exactly 0% of packages in Ruby that I have worked with. Is this not rubocop-packaging? I am trying to understand.

I'll avoid AI in this repo in the future.

@mvz

mvz commented Aug 30, 2026

Copy link
Copy Markdown
Collaborator Author

I have a test fixture ensuring my dev harness (about 70 gems and growing) works on a gem that has a gemspec but no Gemfile.

Great! Two questions:

  • Is this test fixture available somewhere?
  • Do you have an actual example of a gem that has a gemspec but no Gemfile? I'm not trying to be difficult here, I just don't think I've come across any except before Bundler became the default tool for dependency isolation.

That seems like a valid target for a rubocop project concerned with "packaging", since Ruby packages are most frequently rubygems, and gemspecs are part of the explicit profile of a rubygem, while Gemfile/gems.rb is not.

Packaging here refers to packaging by, e.g., Linux distros, so cops are meant to stimulate behaviors that will make it easier to repackage ruby gems as .deb packages and similar. The cops will typically be run inside the project that produces the gem, which in my mind will (nearly?) always have a Gemfile. Again, actual counterexamples are welcome.

There are many theoretical edge cases I can think of with Gemfiles being put somewhere as a fixture, or no Gemfile being present because people use appraisal, or no gemspec being present because it is generated on the fly by jeweler. Because I cannot possibly guess all of them beforehand, I'd like to start with a simple implementation and take it from there.

Eventually, it makes sense to make the package root configurable somehow.

@pboling

pboling commented Aug 30, 2026 •

Copy link
Copy Markdown

The fixture I was referring to is here (inline):
https://github.com/structuredmerge/structuredmerge-ruby/blob/main/gems/kettle-jem/spec/integration/structural_merge_template_spec.rb#L1114-L1158

It's not proving this scenario, rather it proves that my tooling can run on a gem with only a gemspec and no Gemfile, not linting per se.

I found another Gemfile-free fixture gem I use for testing with Appraisals: https://github.com/appraisal-rb/appraisal2/tree/main/spec/fixtures/test_gem

It also is not specifically testing linting integration.

My main claim is this:

Gemfile-only root discovery excludes valid gemspec-root projects

And I think that matters, because Gemfile is not a prescribed part of a published gem, neither is it part of the packages this project would recommend. So an unpacked installed gem would not have a Gemfile.

This project would be unable to recognize the package shape it recommends as a package, and it would be unable to lint it. Worse, if you have a Gemfile in a parent directory it wouldn't even fail. It would likely succeed with a false positive, report a clean lint, having not linted the package at all. It would look right past it to any random parent dir that has a Gemfile.

The gemspec has add_development_dependency for a good reason, and the proposed removal of that directive was rejected for a good reason.

At the same time - no, I do not intend to build gems without Gemfiles. So this is absolutely a storm in a tea kettle.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Package root determination

2 participants