Skip to content

Enhance fixup target to use plutil and get correct xctest path #673

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 2 commits into from
Jun 5, 2020

Conversation

edymtt
Copy link
Contributor

@edymtt edymtt commented Jun 4, 2020

There are two reason to use plutil

  • defaults will no longer support this in the future, as per man page

WARNING: The defaults command will be changed in an upcoming major
release to only operate on preferences domains. General plist
manipulation utilities will be folded into a different command-line
program.

  • plutil prints more meaningful errors that help in troubleshooting CI issues

Indeed this switch helped uncover an issue with the logic inferring the path of XcodePerfTests.xctest.

Addresses rdar://63984731

The reason for this is twofold:
* `defaults` will not longer support this in the future, as per man page

> WARNING: The defaults command will be changed in an upcoming major
release to only operate on preferences domains. General plist
manipulation utilities will be folded into a different command-line
program.
* this may help address some failures in Swift CI related to the
  execution of this command

Addresses rdar://63984731
@edymtt edymtt requested review from ddunbar and dmbryson as code owners June 4, 2020 18:57
@edymtt
Copy link
Contributor Author

edymtt commented Jun 4, 2020

@swift-ci please smoke test

@ddunbar ddunbar changed the title Use defaults instead of plutil to modify plists Use plutil instead of defaults to modify plists Jun 4, 2020
@ddunbar
Copy link
Contributor

ddunbar commented Jun 4, 2020

Thanks for the PR!

Your commit title is backwards. :)

@edymtt
Copy link
Contributor Author

edymtt commented Jun 4, 2020

Thanks for catching my typo 😄

Also add explicit dependency of the fixup target on the bundle target
@edymtt
Copy link
Contributor Author

edymtt commented Jun 5, 2020

@swift-ci please smoke test

@edymtt edymtt changed the title Use plutil instead of defaults to modify plists Enhance fixup phase to use plutil and get correct xctest path Jun 5, 2020
@edymtt edymtt requested review from ddunbar and jakepetroules June 5, 2020 16:31
@edymtt
Copy link
Contributor Author

edymtt commented Jun 5, 2020

Rerequesting reviews since the PR scope is slightly expanded now.

@edymtt edymtt changed the title Enhance fixup phase to use plutil and get correct xctest path Enhance fixup target to use plutil and get correct xctest path Jun 5, 2020
@edymtt
Copy link
Contributor Author

edymtt commented Jun 5, 2020

@swift-ci please smoke test

@dmbryson dmbryson merged commit 145c113 into swiftlang:master Jun 5, 2020
@edymtt edymtt deleted the prefer_plutil_to_defaults branch June 8, 2020 14:16
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