← 输入框|github.com|刷新
Skip to content

Revert "Disambiguate Document.h import" - #694

Open
dtweston wants to merge 2 commits into
SVGKit:3.xfrom
dtweston:revert-690-patch-1
Open

Revert "Disambiguate Document.h import"#694
dtweston wants to merge 2 commits into
SVGKit:3.xfrom
dtweston:revert-690-patch-1

Conversation

@dtweston

@dtweston dtweston commented Sep 9, 2020

Copy link
Copy Markdown
Contributor

Reverts #690

This change breaks the build with an error like:

In file included from /Users/dtweston/src/SVGKit/Source/DOM classes/SVG-DOM/SVGTextPositioningElement.m:4:
In file included from /Users/dtweston/src/SVGKit/Source/DOM classes/Unported or Partial DOM/SVGElement_ForParser.h:3:
In file included from /Users/dtweston/src/SVGKit/Source/Parsers/SVGKParseResult.h:8:
/Users/dtweston/src/SVGKit/Source/DOM classes/SVG-DOM/SVGDocument.h:21:1: error: duplicate interface definition for class 'SVGDocument'
@interface SVGDocument : Document
^
In module 'SVGKit' imported from /Users/dtweston/src/SVGKit/Source/DOM classes/SVG-DOM/SVGDocument.h:18:
/Users/dtweston/src/SVGKit/Source/DOM classes/SVG-DOM/SVGDocument.h:21:12: note: previous definition is here
@interface SVGDocument : Document
           ^

@adamgit

adamgit commented Sep 9, 2020

Copy link
Copy Markdown
Contributor

If it's just Apple failing to notice that it's already seen that interface during this build run - better perhaps to #ifdef / #def the SVGDocument header? (manual #define guard)?

@dtweston

dtweston commented Sep 10, 2020

Copy link
Copy Markdown
Contributor Author

Adding an include guard does also fix the issue. I opened a new pull request #696

While this does fix the issue, it doesn't seem ideal. A better fix might be to disable header maps and use the correct paths everywhere to locate the file to import.

@dtweston dtweston mentioned this pull request Sep 10, 2020
Closed
@dtweston

dtweston commented Sep 10, 2020

Copy link
Copy Markdown
Contributor Author

Adding an include guard does also fix the issue. I opened a new pull request #696

While this does fix the issue, it doesn't seem ideal. A better fix might be to disable header maps and use the correct paths everywhere to locate the file to import.

Whoops, my bad. The include guard doesn't fix the issue. I may have tested it locally with both the revert and the include guard, which made me think it did.

@dtweston
Use relative path to disambiguate instead of framework import
0a3ca32 
The relative path should be sufficient to disambiguate between the SVGKit header
vs. a random other header WITHOUT introducing module errors.
@adamgit

adamgit commented Sep 17, 2020

Copy link
Copy Markdown
Contributor

So, to clarify, current situation is:

  1. Disambiguated headers is working for some platforms, but breaking others
  2. Where it's broken: include-guards aren't fixing it
  3. This PR reverts the disambiguation, but leaves open the possibility of someone attempting disambiguation again in future, if they can demonstrate that their change doesn't re-create these problems

?

If so: for future reference, what platforms are succeeeding with the disambiguation and which are failing? (so we can point any future PRs at this and make sure they get tested against both!)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@dtweston @adamgit