<p dir="auto">I saw a CI failure (<a href="https://github.com/openstreetmap/openstreetmap-website/actions/runs/18712835648/job/53365159955?pr=6464">https://github.com/openstreetmap/openstreetmap-website/actions/runs/18712835648/job/53365159955?pr=6464</a>) that suggested something wrong with my cleanup of <code class="notranslate">require</code> calls at <a class="issue-link js-issue-link" data-error-text="Failed to load title" data-id="3537413598" data-permission-text="Title is private" data-url="https://github.com/openstreetmap/openstreetmap-website/issues/6461" data-hovercard-type="pull_request" data-hovercard-url="/openstreetmap/openstreetmap-website/pull/6461/hovercard" href="https://github.com/openstreetmap/openstreetmap-website/pull/6461">#6461</a>. After looking into it, I think I'm seeing a situation where referencing the <code class="notranslate">XML</code> module can break randomly.</p>
<p dir="auto">Before <a class="issue-link js-issue-link" data-error-text="Failed to load title" data-id="3537413598" data-permission-text="Title is private" data-url="https://github.com/openstreetmap/openstreetmap-website/issues/6461" data-hovercard-type="pull_request" data-hovercard-url="/openstreetmap/openstreetmap-website/pull/6461/hovercard" href="https://github.com/openstreetmap/openstreetmap-website/pull/6461">#6461</a>, there was a <code class="notranslate">require "xml/libxml"</code> in <code class="notranslate">app/models/changeset.rb</code>. This in turn triggered a global <code class="notranslate">include LibXML</code> (see at <a href="https://github.com/xml4r/libxml-ruby/blob/daa51c5174196516be6cd3cb05d4e1c8f9e11ea0/lib/xml/libxml.rb#L9">libxml-ruby</a>) which made <code class="notranslate">LibXML::XML</code> available as simply <code class="notranslate">XML</code>. This then is taken advantage of in multiple places in the codebase.</p>
<p dir="auto">As I see it, the old <code class="notranslate">require</code> 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.</p>
<p dir="auto">My change to remove that <code class="notranslate">require</code> 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 <code class="notranslate">include</code> altogether. This PR does that in two steps:</p>
<ol dir="auto">
<li>Remove a couple of <code class="notranslate">require</code>s that were still triggering the stealth <code class="notranslate">include</code>. This makes evident which locations were referring to <code class="notranslate">XML</code> as a top-level module.</li>
<li>Properly refer to <code class="notranslate">XML</code> as <code class="notranslate">LibXML::XML</code>, or at least have an explicit <code class="notranslate">include LibXML</code> in places where it's more convenient to do so.</li>
</ol>
<hr>
<h4>You can view, comment on, or merge this pull request online at:</h4>
<p> <a href='https://github.com/openstreetmap/openstreetmap-website/pull/6465'>https://github.com/openstreetmap/openstreetmap-website/pull/6465</a></p>
<h4>Commit Summary</h4>
<ul>
<li><a href="https://github.com/openstreetmap/openstreetmap-website/pull/6465/commits/2bcc7fbd3acd9ba976e7984be4dafebf66898467" class="commit-link">2bcc7fb</a> Remove spurious requires that pretend that XML is a top-level module</li>
<li><a href="https://github.com/openstreetmap/openstreetmap-website/pull/6465/commits/2bcbf25450fe3e02b2eb1869dccd4cdf555aa120" class="commit-link">2bcbf25</a> Avoid referencing submodule XML directly</li>
</ul>
<h4 style="display: inline-block">File Changes </h4> <p style="display: inline-block">(<a href="https://github.com/openstreetmap/openstreetmap-website/pull/6465/files">8 files</a>)</p>
<ul>
<li>
<strong>M</strong>
<a href="https://github.com/openstreetmap/openstreetmap-website/pull/6465/files#diff-c264648051a5016bf8ab7161c8a8ac50aa4aee854789e1b327b84b0667ae1429">app/controllers/api/user_preferences_controller.rb</a>
(2)
</li>
<li>
<strong>M</strong>
<a href="https://github.com/openstreetmap/openstreetmap-website/pull/6465/files#diff-766c34fd6533171eaf54300c153f89d6002c35c02cfc9c5b219251f85180ad07">app/controllers/application_controller.rb</a>
(4)
</li>
<li>
<strong>M</strong>
<a href="https://github.com/openstreetmap/openstreetmap-website/pull/6465/files#diff-213f2b17d6c8732fec135ce33c196a75f1733c968bceb998416c992333f33fad">app/jobs/trace_importer_job.rb</a>
(2)
</li>
<li>
<strong>M</strong>
<a href="https://github.com/openstreetmap/openstreetmap-website/pull/6465/files#diff-770a3485a6eafabd9855d92aca8ab0ad055aee57c44a713a10221fdb67b65cfb">app/models/application_record.rb</a>
(2)
</li>
<li>
<strong>M</strong>
<a href="https://github.com/openstreetmap/openstreetmap-website/pull/6465/files#diff-a648e9469f14900f40ca27b837ba17cc9bb53c93721295889e297489b1ae41dd">lib/diff_reader.rb</a>
(2)
</li>
<li>
<strong>M</strong>
<a href="https://github.com/openstreetmap/openstreetmap-website/pull/6465/files#diff-2f101bb482816df81f20afe2e8626836bd542158be8744474dcffcc4a113d769">lib/gpx.rb</a>
(2)
</li>
<li>
<strong>M</strong>
<a href="https://github.com/openstreetmap/openstreetmap-website/pull/6465/files#diff-f8a27e50423ab8e432a0ac66073c425c0c4eb520ecd92bf152b21e6e966b4754">lib/osm.rb</a>
(7)
</li>
<li>
<strong>M</strong>
<a href="https://github.com/openstreetmap/openstreetmap-website/pull/6465/files#diff-ba37813ca277c227a74a372479b7b05b7f3ff085d890ab708f80d62573efdb7a">test/test_helper.rb</a>
(1)
</li>
</ul>
<h4>Patch Links:</h4>
<ul>
<li><a href='https://github.com/openstreetmap/openstreetmap-website/pull/6465.patch'>https://github.com/openstreetmap/openstreetmap-website/pull/6465.patch</a></li>
<li><a href='https://github.com/openstreetmap/openstreetmap-website/pull/6465.diff'>https://github.com/openstreetmap/openstreetmap-website/pull/6465.diff</a></li>
</ul>
<p style="font-size:small;-webkit-text-size-adjust:none;color:#666;">—<br />Reply to this email directly, <a href="https://github.com/openstreetmap/openstreetmap-website/pull/6465">view it on GitHub</a>, or <a href="https://github.com/notifications/unsubscribe-auth/AAK2OLMULLGUWRT5PBVNR2L3Y5VCBAVCNFSM6AAAAACJ4MCT42VHI2DSMVQWIX3LMV43ASLTON2WKOZTGU2DANJSHAZDAMI">unsubscribe</a>.<br />You are receiving this because you are subscribed to this thread.<img src="https://github.com/notifications/beacon/AAK2OLIJ4GLWQUJ5UOHHUAT3Y5VCBA5CNFSM6AAAAACJ4MCT42WGG33NNVSW45C7OR4XAZNFJFZXG5LFVJRW63LNMVXHIX3JMTHNGCBMJE.gif" height="1" width="1" alt="" /><span style="color: transparent; font-size: 0; display: none; visibility: hidden; overflow: hidden; opacity: 0; width: 0; height: 0; max-width: 0; max-height: 0; mso-hide: all">Message ID: <span><openstreetmap/openstreetmap-website/pull/6465</span><span>@</span><span>github</span><span>.</span><span>com></span></span></p>
<script type="application/ld+json">[
{
"@context": "http://schema.org",
"@type": "EmailMessage",
"potentialAction": {
"@type": "ViewAction",
"target": "https://github.com/openstreetmap/openstreetmap-website/pull/6465",
"url": "https://github.com/openstreetmap/openstreetmap-website/pull/6465",
"name": "View Pull Request"
},
"description": "View this Pull Request on GitHub",
"publisher": {
"@type": "Organization",
"name": "GitHub",
"url": "https://github.com"
}
}
]</script>