1776104 - Verify that the paths in ThirdPartyPaths.txt exist before adding them to ThirdPartyPaths.cpp

Status: NEW

(Developer Infrastructure :: Source Code Analysis, enhancement)

Description

[Ryan VanderMeulen [:RyanVM]]

When looking at ThirdPartyPaths.txt recently, I noticed a least a couple paths which no longer exist. We should add a check somewhere which verifies the paths listed. Maybe with os.path.exists() in ThirdPartyPaths.py before it writes ThirdPartyPaths.cpp?

Comment 1

[Ryan VanderMeulen [:RyanVM]]

I have patches which mostly work (and correctly identified 16 paths which didn't actually exist), with one problem:

/home/ryanvm/repos/mozilla/obj-x86_64-pc-linux-gnu/dist/include/function2/function2.hpp:1595:30: error: bad implicit conversion constructor for 'function'

Somewhere along the way, function2 moved from topsrcdir to a sub-directory in third_party. I guess the previous (technically incorrect) entry was still enough to avoid the error from hitting in the exported header, but now that's not sufficient anymore. Any thoughts on how to handle this, glandium? I guess technically anything in third_party is susceptible to this kind of problem.

Comment 2

[Ryan VanderMeulen [:RyanVM]]

Attachment: Bug 1776104 - Verify that the paths exist before writing ThirdPartyPaths.cpp. r=andi

Comment 3

[Ryan VanderMeulen [:RyanVM]]

Attachment: Bug 1776104 - Remove missing paths from Generated.txt and ThirdPartyPaths.txt. r=andi

Also refresh the list generated from ThirdPartyPaths.txt in
.clang-format-ignore.

Depends on D150069

Comment 4

[Ryan VanderMeulen [:RyanVM]]

I finally got a working solution to the function2 problem brought up in comment 1. Basically, it requires providing a mechanism for special-case overrides of this check for situations that warrant it. I also created a third list type, Unvalidated.txt, to avoid issues with any other consumers of ThirdPartyPaths.txt. With this update, we effectively treat function2 exactly as it was prior to my patches and everything's green on Try.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/05da7d6fa999
Verify that the paths exist before writing ThirdPartyPaths.cpp. r=andi
https://hg.mozilla.org/integration/autoland/rev/c2f124ed65ec
Remove missing paths from Generated.txt and ThirdPartyPaths.txt. r=andi

Comment 6

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/05da7d6fa999
https://hg.mozilla.org/mozilla-central/rev/c2f124ed65ec

Comment 7

[Narcis Beleuzu [:NarcisB]]

Backed out as requested by Andi

Backout link: https://hg.mozilla.org/integration/autoland/rev/2f920593c4f1af0a0bf1e0b8a9d06b005fb666d1

Comment 8

[Andi [:andi]]

We had to backout the patches because it caused clang-tidy to fail since it's built our of the tree.

Comment 9

[Atila Butkovits]

Backout merged to central: https://hg.mozilla.org/mozilla-central/rev/2f920593c4f1af0a0bf1e0b8a9d06b005fb666d1

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Comment 8 doesn't leave me much to go off with respect to how to address the issue that led to the backout. Can you please elaborate?

Comment 11

[Ryan VanderMeulen [:RyanVM]]

This would probably be better off handled as a new lint instead. Maybe bug 1879254 gives some ideas to build on for doing so.