Bug 198008 - generate-xcfilelists is stranding temporary files
Summary: generate-xcfilelists is stranding temporary files
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Keith Rollin
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-05-17 16:46 PDT by Keith Rollin
Modified: 2019-05-20 11:24 PDT (History)
5 users (show)

See Also:


Attachments
Patch (7.89 KB, patch)
2019-05-17 16:49 PDT, Keith Rollin
no flags Details | Formatted Diff | Diff
Patch (7.93 KB, patch)
2019-05-20 09:48 PDT, Keith Rollin
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Keith Rollin 2019-05-17 16:46:47 PDT
generate-xcfilelists makes use of temporary files on disk. These files are opened with the OS's "temporary" bit set, causing them to get deleted when closed or the process exists. However, these temporary files actually end up persisting after the script exists. This is because `sed` is used to process the files, and is done so in a way that causes the "temporary" bit to get cleared.

Address this issue by no longer using `sed` and instead performing the equivalent processing the file content in-memory.

rdar://problem/50893659
Comment 1 Keith Rollin 2019-05-17 16:49:20 PDT
Created attachment 370169 [details]
Patch
Comment 2 Build Bot 2019-05-17 16:51:58 PDT
Attachment 370169 [details] did not pass style-queue:


ERROR: Tools/Scripts/webkitpy/generate_xcfilelists_lib/generators.py:368:  expected 1 blank line, found 0  [pep8/E301] [5]
Total errors found: 1 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Jonathan Bedard 2019-05-20 08:32:33 PDT
Comment on attachment 370169 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=370169&action=review

> Tools/Scripts/webkitpy/generate_xcfilelists_lib/generators.py:346
> +        return set([re.sub(to_replace, replace_with, line) for line in lines])

Putting these in a set will throw out any order you have, not sure if that's an OK side-effect here....it might not have correctness problems, but I could see it making the xcfilelists less readable.

> Tools/Scripts/webkitpy/generate_xcfilelists_lib/generators.py:368
> +        def get_lines(source):

The style checker is complaining about no newlines surrounding this. Given how you're using it, I wonder if this:

get_lines = lambda source: source if isinstance(source, set) else set(source) if isinstance(source, list) else self._get_file_lines(source)

would be more readable (and make the style checker happy)
Comment 4 Jonathan Bedard 2019-05-20 08:35:58 PDT
(In reply to Jonathan Bedard from comment #3)
> Comment on attachment 370169 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=370169&action=review
> 
> > Tools/Scripts/webkitpy/generate_xcfilelists_lib/generators.py:346
> > +        return set([re.sub(to_replace, replace_with, line) for line in lines])
> 
> Putting these in a set will throw out any order you have, not sure if that's
> an OK side-effect here....it might not have correctness problems, but I
> could see it making the xcfilelists less readable.

Actually, what we probably want to do is put everything in a set, then convert the set to a list, then sort the list.

> ...
Comment 5 Keith Rollin 2019-05-20 09:13:44 PDT
(In reply to Jonathan Bedard from comment #4)

> Actually, what we probably want to do is put everything in a set, then
> convert the set to a list, then sort the list.

That's exactly what I do. See _merge_added_lines.
Comment 6 Keith Rollin 2019-05-20 09:48:10 PDT
Created attachment 370257 [details]
Patch
Comment 7 Keith Rollin 2019-05-20 09:48:29 PDT
Added blank lines.
Comment 8 WebKit Commit Bot 2019-05-20 11:24:49 PDT
Comment on attachment 370257 [details]
Patch

Clearing flags on attachment: 370257

Committed r245531: <https://trac.webkit.org/changeset/245531>
Comment 9 WebKit Commit Bot 2019-05-20 11:24:50 PDT
All reviewed patches have been landed.  Closing bug.