I saw a CI failure (https://github.com/openstreetmap/openstreetmap-website/actions/runs/18712835648/job/53365159955?pr=6464) that suggested something wrong with my cleanup of `require` calls at https://github.com/openstreetmap/openstreetmap-website/pull/6461. After looking into it, I think I'm seeing a situation where referencing the `XML` module can break randomly.
Before #6461, there was a `require "xml/libxml"` in `app/models/changeset.rb`. This in turn triggered a global `include LibXML` (see at [libxml-ruby](https://github.com/xml4r/libxml-ruby/blob/daa51c5174196516be6cd3cb05d4e1c8f9e11ea0/lib/xml/libxml.rb#L9)) which made `LibXML::XML` available as simply `XML`. This then is taken advantage of in multiple places in the codebase. As I see it, the old `require` wasn't really great, as it loaded something in an unsuspecting part of the code which then was expected to be loaded elsewhere. If things are loaded in the wrong order, this might lead to random breaks. Perhaps there were flaky tests due to this? This appears to be more likely in older versions of Ruby. My change to remove that `require` may have made the issue more acute. Still I think it's the right thing to do, and the correct fix would be to avoid the stealth `include` altogether. This PR does that in two steps: 1. Remove a couple of `require`s that were still triggering the stealth `include`. This makes evident which locations were referring to `XML` as a top-level module. 2. Properly refer to `XML` as `LibXML::XML`, or at least have an explicit `include LibXML` in places where it's more convenient to do so. You can view, comment on, or merge this pull request online at: https://github.com/openstreetmap/openstreetmap-website/pull/6465 -- Commit Summary -- * Remove spurious requires that pretend that XML is a top-level module * Avoid referencing submodule XML directly -- File Changes -- M app/controllers/api/user_preferences_controller.rb (2) M app/controllers/application_controller.rb (4) M app/jobs/trace_importer_job.rb (2) M app/models/application_record.rb (2) M lib/diff_reader.rb (2) M lib/gpx.rb (2) M lib/osm.rb (7) M test/test_helper.rb (1) -- Patch Links -- https://github.com/openstreetmap/openstreetmap-website/pull/6465.patch https://github.com/openstreetmap/openstreetmap-website/pull/6465.diff -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/6465 You are receiving this because you are subscribed to this thread. Message ID: <openstreetmap/openstreetmap-website/pull/[email protected]>
_______________________________________________ rails-dev mailing list [email protected] https://lists.openstreetmap.org/listinfo/rails-dev
