1438866 - Make mozsearch c++ indexer more windows-friendly

Status: RESOLVED FIXED

(Webtools :: Searchfox, enhancement)

Description

[Kartikaya Gupta (email:[e-mail])]

I have some patches that make the c++ indexer more windows friendly. i.e. they don't cause clang to immediately crash. This is needed for bug 1418415.

Comment 1

[Kartikaya Gupta (email:[e-mail])]

Attachment: Bug 1438866 - Use proper path separators for windows.

The indexer has paths handed to it on Windows with the backslash path separator.
However it currently hard-codes the forward-slash Unix/macOS path separator,
so we need to generify that code appropriately.

Review commit: https://reviewboard.mozilla.org/r/220930/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/220930/

Comment 2

[Kartikaya Gupta (email:[e-mail])]

Attachment: Bug 1438866 - Add some utility code to help with debugging.

This adds an RAII class and macro that can be quickly added in functions
to log entry/exit from the function. This is useful to debugging.

Review commit: https://reviewboard.mozilla.org/r/220932/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/220932/

Comment 3

[Kartikaya Gupta (email:[e-mail])]

Attachment: Bug 1438866 - Mark empty filenames not interesting.

In some cases we have SourceLocation objections that are isMacroId()
rather than isFileId() and so don't have a filename at all. In other
cases the filename is something clang-internal like "<scratch>". In
both of these situations we don't want to output any analysis data.
However, the code previously was taking the empty filename and resolving
it relative to the source folder, and then tried to write to the
source folder or create a file corresponding to a subfolder. This
resulted in general badness. This patch makes sure we ignore analysis
from places that don't have a corresponding file so that we don't
have this problem.

Review commit: https://reviewboard.mozilla.org/r/220934/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/220934/

Comment 4

[Kartikaya Gupta (email:[e-mail])]

I verified these patches don't affect the output on Linux, by comparing the contents of the the target.mozsearch-index.zip files from my try push [1] and the base revision [2].

[1] https://treeherder.mozilla.org/#/jobs?repo=try&revision=29fde2456ba99b0e9b9df6729f2085bb416ed323&selectedJob=162544224
[2] https://treeherder.mozilla.org/#/jobs?repo=mozilla-central&revision=9b69cc60e5848f2f8802c911fd00771b50eed41f&filter-searchStr=searchfox&selectedJob=162375224

Comment 5

[Kartikaya Gupta (email:[e-mail])]

I also verified these patches successfully index m-c for me locally on a windows box.

Comment 6

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 8951641 [details]
Bug 1438866 - Use proper path separators for windows.

https://reviewboard.mozilla.org/r/220930/#review226956

Comment 7

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 8951642 [details]
Bug 1438866 - Add some utility code to help with debugging.

https://reviewboard.mozilla.org/r/220932/#review226958

Not super-fan of adding debug-only code that is dead, but I guess as long as it's useful... it's fine. r=me

Comment 8

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 8951643 [details]
Bug 1438866 - Mark empty filenames not interesting.

https://reviewboard.mozilla.org/r/220934/#review226960

::: build/clang-plugin/mozsearch-plugin/MozsearchIndexer.cpp:180
(Diff revision 1)
>      It = FileMap.find(Id);
>      if (It == FileMap.end()) {
>        // We haven't seen this file before. We need to make the FileInfo
>        // structure information ourselves
>        std::string Filename = SM.getFilename(Loc);
> -      std::string Absolute = getAbsolutePath(Filename);
> +      std::string Absolute;

Probably worth a comment in both places?

Comment 9

[Kartikaya Gupta (email:[e-mail])]

Added comments

Comment 10

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8951641 [details]
Bug 1438866 - Use proper path separators for windows.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/220930/diff/1-2/

Comment 11

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8951642 [details]
Bug 1438866 - Add some utility code to help with debugging.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/220932/diff/1-2/

Comment 12

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8951643 [details]
Bug 1438866 - Mark empty filenames not interesting.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/220934/diff/1-2/

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a04f505f9454
Use proper path separators for windows. r=emilio
https://hg.mozilla.org/integration/autoland/rev/a42f1a25f088
Add some utility code to help with debugging. r=emilio
https://hg.mozilla.org/integration/autoland/rev/d071f1a3ace4
Mark empty filenames not interesting. r=emilio

Comment 14

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/a04f505f9454
https://hg.mozilla.org/mozilla-central/rev/a42f1a25f088
https://hg.mozilla.org/mozilla-central/rev/d071f1a3ace4