Bug 78067 – [chromium] Move geometry headers in Platform API to Platform directory

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[James Robinson]

[chromium] Move geometry headers in Platform API to Platform directory

[James Robinson]

Created attachment 125989 [details]
Patch

[WebKit Review Bot]

Please wait for approval from [e-mail] before submitting because this patch contains changes to the Chromium public API.

[James Robinson]

Created attachment 125991 [details]
Patch

[WebKit Review Bot]

Attachment 125991 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/Platform/ChangeLog', u'Source/Platf..." exit_code: 1

Source/Platform/chromium/src/WebFloatQuad.cpp:34:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 1 in 18 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Adam Barth]

Comment on attachment 125991 [details]
Patch

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

We've already talked this over with fishd and we're not actually changing anything in the API.  It's probably safe to land without round-tripping with him.

> Source/Platform/Platform.gyp/copy_webcore_headers.py:37
> +            raise e

You just want to say "raise" without the "e"

[James Robinson]

Comment on attachment 125991 [details]
Patch

Whoops missed the comment, will update

[James Robinson]

Created attachment 126002 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 126002 [details]
Patch for landing

Clearing flags on attachment: 126002

Committed r107042: <http://trac.webkit.org/changeset/107042>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Sadrul Habib Chowdhury]

Reopening to attach new patch.

[Sadrul Habib Chowdhury]

Created attachment 126101 [details]
Patch

[Sadrul Habib Chowdhury]

(In reply to comment #10)
> Reopening to attach new patch.

My bad. I failed webkit-patch

[Tony Chang]

Comment on attachment 126002 [details]
Patch for landing

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

> Source/Platform/Platform.gyp/Platform.gyp:58
> +                # Since Platform/ can't add WebCore/* to the include path, this build step
> +                # copies these headers into the shared intermediate directory and adds that to the include path.

Copying the headers is OK, but forwarding headers are better.  Sometimes people edit the wrong file by accident when there's a copy.

[James Robinson]

(In reply to comment #13)
> (From update of attachment 126002 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=126002&action=review
> 
> > Source/Platform/Platform.gyp/Platform.gyp:58
> > +                # Since Platform/ can't add WebCore/* to the include path, this build step
> > +                # copies these headers into the shared intermediate directory and adds that to the include path.
> 
> Copying the headers is OK, but forwarding headers are better.  Sometimes people edit the wrong file by accident when there's a copy.

I disagree - forwarding headers run into all sorts of gross issues with relative paths and I really doubt anyone is going to accidentally edit a copy in out/Release/gen/webcore_headers/ instead of the real one.

[Tony Chang]

(In reply to comment #14)
> (In reply to comment #13)
> > (From update of attachment 126002 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=126002&action=review
> > 
> > > Source/Platform/Platform.gyp/Platform.gyp:58
> > > +                # Since Platform/ can't add WebCore/* to the include path, this build step
> > > +                # copies these headers into the shared intermediate directory and adds that to the include path.
> > 
> > Copying the headers is OK, but forwarding headers are better.  Sometimes people edit the wrong file by accident when there's a copy.
> 
> I disagree - forwarding headers run into all sorts of gross issues with relative paths and I really doubt anyone is going to accidentally edit a copy in out/Release/gen/webcore_headers/ instead of the real one.

People who use an IDE just right click on an include an open it.  It's easy to accidentally edit the wrong copy.  It's also not obvious where copies come from.

Here's a previous discussion where it confused people (which is when I switched the third_party webkit headers to use forwarding headers from a copy):
https://groups.google.com/a/chromium.org/group/chromium-dev/browse_thread/thread/97f633fea1369fa/b42a06a0b5deea90?lnk=gst

[James Robinson]

That's a little different, in that case both copies were in the source directories. In this case the copy is in the SHARED_INTERMEDIATE_DIR. We should just teach IDEs not to open files in the output directory, if we haven't already.

I don't think anyone is using the xcode UI any more (at least not if they've upgraded to 4.X).

[Tony Chang]

(In reply to comment #16)
> That's a little different, in that case both copies were in the source directories. In this case the copy is in the SHARED_INTERMEDIATE_DIR. We should just teach IDEs not to open files in the output directory, if we haven't already.

How do we teach IDEs not to open files in the output directory?

> I don't think anyone is using the xcode UI any more (at least not if they've upgraded to 4.X).

The same happens in MSVS.  If you open a header, it only has the filename, not the full path in the tab.  It's easy to edit the wrong file.