Skip to content

[windows] Replace SourceKit cleaning regexes for Python script. #28298

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

Merged
merged 1 commit into from
Nov 20, 2019

Conversation

drodriguez
Copy link
Contributor

The quoting of the sed commands was creating problems in my Windows
installation. I am unsure if the implementation of sed.exe is different
or the cmd.exe is different.

In order to avoid problems in different machines, replace the piped sed
commands into only one python script. This should be multiplatform and
should execute the same in any of them. It also remove a lot of the
extra quoting and escaping, and avoids 5 processes for only just one.

Copy link
Contributor

@gmittert gmittert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm always happy to have less shelling out to sed. Since as far as I can tell, the new python regexes match the old sed ones, this looks good to me.

@drodriguez
Copy link
Contributor Author

@swift-ci please smoke test

Copy link
Member

@compnerd compnerd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 launch is way better than the piped mess. The matches look equivalent to me. Nice clean up :)

@compnerd
Copy link
Member

@swift-ci please test Windows platform

Copy link
Contributor

@akyrtzi akyrtzi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@compnerd
Copy link
Member

@swift-ci please smoke test macOS platform

@drodriguez
Copy link
Contributor Author

It’s failing in the Python linter. I will have a look and fix it when I have time.

The quoting of the sed commands was creating problems in my Windows
installation. I am unsure if the implementation of sed.exe is different
or the cmd.exe is different.

In order to avoid problems in different machines, replace the piped sed
commands into only one python script. This should be multiplatform and
should execute the same in any of them. It also remove a lot of the
extra quoting and escaping, and avoids 5 processes for only just one.
@drodriguez drodriguez force-pushed the windows-python-sed_clean branch from d468948 to 8470083 Compare November 19, 2019 00:51
@drodriguez
Copy link
Contributor Author

@swift-ci please smoke test

@drodriguez
Copy link
Contributor Author

drodriguez commented Nov 19, 2019

@swift-ci please test Windows platform

[Edit: https://ci-external.swift.org/view/Pull%20Request/job/swift-PR-windows/197/]

@drodriguez drodriguez merged commit 4cd873a into swiftlang:master Nov 20, 2019
@drodriguez drodriguez deleted the windows-python-sed_clean branch November 20, 2019 21:50
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.

4 participants