(PUP-11597) Generate types when any module libs are updated

[natemccurdy]

Fixes PUP-11597

Before

Prior to this, the puppet generate types command would only detect resource type changes if the type file itself changed.

For example, if your type was defined at <module>/lib/puppet/type/foo.rb, a change in it would be detected.

But if your type includes libraries from separate Ruby files in lib/puppet/* or even in the PuppetX namespace like lib/puppet_x/*, changes to those other files would not be detected.

The consequence of that behavior is that updates to the signature of a type that come from "other" ruby files would not trigger a metadata re-creation and those types would suffer from the environment-isolation problem (SERVER-94) that puppet generate types was meant to solve.

After

This fixes that problem by updating puppet generate types to scan for changed Ruby files within a module's lib/* directory.

It uses the same Puppet::FileSystem::stat operation to compare mtime of the Ruby files in lib/* relative to the generated metadata file. And if the metadata is older than any of the Ruby files in lib/*, the metadata is regenerated.

The one slight-downside to my solution is that generation of type metadata will possibly happen more often than before and possibly more often than is needed. However, generating types is fast, and it's usually only done when code updates happen (e.g. via r10k or Code Manager), so the total added load is very close to, if not, negligible.

[puppetlabs-jenkins]

Can one of the admins verify this patch?

[natemccurdy]

I'm not sure why the spec tests are failing in jruby and windows. Anyone have an idea why?

[joshcooper]

Since CI failed more than a month ago, I have to close and reopen to rekick the GH actions.

[joshcooper]

@natemccurdy I think Windows tests are failing because of the way the test stubs filesystem access. Perhaps Windows mtime resolution isn't as low as Posix or the time is being truncated?

  1) Puppet::Face[:generate, 0.1.0] when used from an interactive terminal in an environment with two modules containing resource types overwrites if files exists that are not up to date while keeping up to date files
     Failure/Error: expect(stats_before[0] <=> stats_after[0]).to eq(-1)
       expected: -1
            got: 0
       (compared using ==)
```. 

[joshcooper]

Just noting that this needs work

[natemccurdy]

I wasn't able to figure out a way to get the tests to pass on Windows by faking a passage of time, so I resorted to a sleep(1) which is what the existing tests do.

I don't like that sleep, but the tests pass 🤷 .

[natemccurdy]

@joshcooper Is there anything specific here that still needs work?

[CLAassistant]

All committers have signed the CLA.

[joshcooper]

Retriggering tests

[joshcooper]

That is surprising because our filesystem code calls FileUtils.touch which eventually passes the mtime through to https://github.com/ruby/ruby/blob/536649f819ed8f2bb0f8f44b1a0ca5c6d1753b24/win32/win32.c#L7692 where SetFileTime is documented in https://learn.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-setfiletime

[natemccurdy]

I tried a build just now that uses:

Puppet::FileSystem.touch(puppet_x_lib, :mtime => Time.now + 1000)

It failed in the jruby spec tests: https://github.com/puppetlabs/puppet/actions/runs/6882098596/job/18719877970

From what I remember when I first made this PR, the sleep(1) was the only way to get the tests to pass.
Though now that I look at it, the comment about "doesn't work on Windows" might be wrong... it's actually failing on Jruby and the Windows tests are being skipped because of that.

[joshcooper]

Wondering if it'd be better to use the block form and break as soon as we detect a newer file?

-         module_lib_files = Dir.glob(File.join(@base, "lib", "**", "*.rb"))
-         module_lib_files.each do |lib|
+         Dir.glob(File.join(@base, "lib", "**", "*.rb")) do |lib|

Also I'd want to understand how this would affect code deployment times for a "typical" environment with hundreds of modules.