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.

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

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

Open
wants to merge 1 commit into
base: 7.x
Choose a base branch
from
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
18 changes: 17 additions & 1 deletion lib/puppet/generate/type.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,23 @@ def format=(format)
# @return [Boolean] Returns true if the output is up-to-date or false if not.
def up_to_date?(outputdir)
f = effective_output_path(outputdir)
Puppet::FileSystem::exist?(f) && (Puppet::FileSystem::stat(@path) <=> Puppet::FileSystem::stat(f)) <= 0
# Check the fast-path scenarios first.
unless Puppet::FileSystem::exist?(f)
Puppet.debug{"#{f} does not exist."}
return false
end
unless (Puppet::FileSystem::stat(@path) <=> Puppet::FileSystem::stat(f)) <= 0
Puppet.debug{"#{@path} is newer than #{f}"}
return false
end
# Check for updates to any module lib files.
Dir.glob(File.join(@base, "lib", "**", "*.rb")) do |lib|
unless (Puppet::FileSystem::stat(lib) <=> Puppet::FileSystem::stat(f)) <= 0
Puppet.debug{"#{lib} is newer than #{f}"}
return false
end
end
return true
end

# Gets the filename of the output file.
Expand Down
16 changes: 15 additions & 1 deletion spec/unit/face/generate_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ module Puppet
} }
},
},
'm2' => {
'm2' => {
'lib' => { 'puppet' => { 'type' => {
'test2.rb' => <<-EOF
module Puppet
Expand Down Expand Up @@ -220,6 +220,20 @@ module Puppet
expect(stat_before <=> stats_after).to be(0)
end

it 'overwrites all files if ruby files in lib/puppet_x/ are updated' do
# create them (first run)
puppet_x_lib = File.join(m1, 'lib', 'puppet_x', 'foo', 'library.rb')
Puppet::FileSystem.mkpath(puppet_x_lib)
genface.types
stats_before = [Puppet::FileSystem.stat(File.join(outputdir, 'test1.pp')), Puppet::FileSystem.stat(File.join(outputdir, 'test2.pp'))]
# Sorry. The sleep is needed because Puppet::FileSystem.touch(<path>, :mtime => Time.now + 1000) doesn't work on Windows.
sleep(1)
Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Puppet::FileSystem.touch(puppet_x_lib)
genface.types
stats_after = [Puppet::FileSystem.stat(File.join(outputdir, 'test1.pp')), Puppet::FileSystem.stat(File.join(outputdir, 'test2.pp'))]
expect(stats_before <=> stats_after).to eq(-1)
end

end

context "in an environment with a faulty type" do
Expand Down