Skip to content
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

[5.9][ScanDependencies] Fix JSON generation under certain circunstances. #67265

Merged
merged 1 commit into from
Jul 14, 2023

Conversation

drodriguez
Copy link
Collaborator

@drodriguez drodriguez commented Jul 12, 2023

Cherry-pick of #67246

  • Release: Swift 5.9
  • Explanation: Cherry-picks a bug fix for the dependency scanner JSON output, which might be invalid in some cases.
  • Scope of Issue: The problem affects users that parse the JSON output generated by the --scan-dependencies. It happens a couple of times in the test suite, and it happens organically when trying to --scan-dependencies of all frameworks and modules in Xcode 15 betas SDKs.
  • Risk: The changes in the test files should be zero risk and should make release/5.9 less prone of introducing JSON formatting bugs by mistake. The change in the code is low risk and its impact should only be limited to the output of --scan-dependencies.
  • Reviewed By: @artemcm

Original commit message:

The code of ScanDependencies.cpp was creating invalid JSON since #66031 because in the case of having extraPcmArgs and swiftOverlayDependencies, but not bridgingHeader, a comma will not be added at the end of extraPcmArgs, creating an invalid JSON file. Additionally that same PR added a trailing comma at the end of the swiftOverlayDependencies, which valid JSON does not allow, but that bug was removed in #66366.

Both problems are, however, present in the 5.9 branch, because #66936 included #66031, but not #66366.

Besides fixing the problem in ScanDependencies.cpp I modified every test that uses --scan-dependencies to pass the produced JSON through Python's json.tool in order to validate proper JSON is produced. In most cases I was able to pipe the output of the tool into FileCheck, but in some cases the validation is done by itself because the checks depend on the exact format generated by --scan-dependencies. In a couple of tests I added a call to FileCheck that seemed to be missing.

Without these changes, two tests seems to be generating invalid JSON in my machine:

  • ScanDependencies/local_cache_consistency.swift (which outputs Expecting ',' delimiter: line 525 column 11 (char 22799))
  • ScanDependencies/placholder_overlay_deps.swift

Additional changes for the cherry-pick:

  • Removed some changes in some CAS tests that are not present in release/5.9.
  • Switch trailingComma from true to false for swiftOverlayDependencies similar to what [CAS] swift dependency scanning using CAS for compiler caching #66366 did in main.
  • Remove the new RUN line in optional_deps_of_testable_imports.swift because it fails in 5.9 if the CHECK is performed.

The code of `ScanDependencies.cpp` was creating invalid JSON since apple#66031
because in the case of having `extraPcmArgs` and `swiftOverlayDependencies`,
but not `bridgingHeader`, a comma will not be added at the end of
`extraPcmArgs`, creating an invalid JSON file. Additionally that same PR
added a trailing comma at the end of the `swiftOverlayDependencies`, which
valid JSON does not allow, but that bug was removed in apple#66366.

Both problems are, however, present in the 5.9 branch, because apple#66936
included apple#66031, but not apple#66366.

Besides fixing the problem in `ScanDependencies.cpp` I modified every test
that uses `--scan-dependencies` to pass the produced JSON through
Python's `json.tool` in order to validate proper JSON is produced. In
most cases I was able to pipe the output of the tool into `FileCheck`,
but in some cases the validation is done by itself because the checks
depend on the exact format generated by `--scan-dependencies`. In
a couple of tests I added a call to `FileCheck` that seemed to be
missing.

Without these changes, two tests seems to be generating invalid JSON in
my machine:

- `ScanDependencies/local_cache_consistency.swift` (which outputs `Expecting ',' delimiter: line 525 column 11 (char 22799)`)
- `ScanDependencies/placholder_overlay_deps.swift`

Additional changes for the cherry-pick:

- Removed some changes in some CAS tests that are not present in
  release/5.9.
- Switch `trailingComma` from `true` to `false` for
  `swiftOverlayDependencies` similar to what apple#66366 did in `main`.
- Remove the new `RUN` line in `optional_deps_of_testable_imports.swift`
  because it fails in 5.9 if the `CHECK` is performed.

(cherry picked from commit 7de1089)
@drodriguez drodriguez requested a review from a team as a code owner July 12, 2023 19:07
@drodriguez
Copy link
Collaborator Author

@swift-ci please test

@artemcm artemcm requested a review from nkcsgexi July 12, 2023 19:52
@drodriguez drodriguez merged commit d77c387 into apple:release/5.9 Jul 14, 2023
5 checks passed
@drodriguez drodriguez deleted the validate-json-5.9 branch July 14, 2023 01:22
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.

None yet

2 participants