Skip to content

Adding SwiftSyntaxBuilder to the build process #379

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 5 commits into from

Conversation

vrjuliao
Copy link
Contributor

@vrjuliao vrjuliao commented Apr 6, 2022

The build process of SwiftSyntax, should build three shared objects:

  • SwiftSyntax
  • SwiftSyntaxParser
  • SwiftSyntaxBuilder

The SwiftSyntaxBuilder one has not been called to be built, then this pull request adds such build call to the build-script.py file. Also, there are two points in the build process that was producing erros during the SwiftSyntaxBuilder build.

  1. Python3 strings does not support string_escape encode. However, python2 strings supports encoding for utf-8 and decoding as unicode_escape.
    I'm not sure about differences between string.encode('utf-8').decode('unicode_escape') and string.decode('string_escape') in Python2, but looking for the data that will be used to fill Tokens.swift.gyb we only have UTF-8 characters, and for that case these approaches are equivalent. See the values in the file swift/utils/gyb_syntax_support/Token.py.
  2. The local module import in SyntaxBuildableWrappers.py and __init__.py is failing. The dot must to be added to tell to pyhton3 interpreter that we are dealing with the local module (actually, the same directory).

vrjuliao and others added 3 commits April 6, 2022 17:22
This script does not build SwiftSyntaxBuilder by default. This change has to be followed by an update in `.gyb` files.
@vrjuliao vrjuliao marked this pull request as ready for review April 6, 2022 21:06
@vrjuliao vrjuliao requested a review from ahoppen as a code owner April 6, 2022 21:06
@ahoppen
Copy link
Member

ahoppen commented Apr 7, 2022

Thanks for your PR 👍. Building SwiftSyntaxBuilder using build-script.py makes sense to me. Currently, we only build it using build-script.py when running tests.

As for your other changes: Those issues are because we are still using Python 2 for all sorts of Python scripts in the swift project. There’s another PR up at #378 that fixes this. I think for this PR we should either

  • Remove your modifications (only keep builder.build("SwiftSyntaxBuilder")) or
  • Wait for Migrate to use Python 3 #378 to be merged before merging this one (which will probably also render your modifications unnecessary)

@vrjuliao
Copy link
Contributor Author

vrjuliao commented Apr 7, 2022

HI @ahoppen!
I didn't see the pr #378 . I'm sorry about that. I will get this modifications back, and only keep the SwiftSyntaxBuilder one.

@ahoppen
Copy link
Member

ahoppen commented Apr 7, 2022

No worries. Would you mind squashing your changes into one commit and force pushing. Having five commits do and undo some changes just looks noisy in the git history.

@kimdv
Copy link
Contributor

kimdv commented May 27, 2022

This have been fixed in #381 🎉

@kimdv
Copy link
Contributor

kimdv commented Jun 16, 2022

@ahoppen think we just can close this one.
#379 (comment)

@ahoppen
Copy link
Member

ahoppen commented Jun 16, 2022

Yes, you’re right 👍

@ahoppen ahoppen closed this Jun 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants