[5.6] Add parser library for 0.50600.0

[keith]

This cherry picks this commit 0467d94 into the 5.6 release branch so that if you're building on the branch you get the same behavior as if you use the tag.

[keith]

This doesn't actually change the release since this is included in the tag already, but I guess if there is another 5.6.x this should probably be included or updated?

[ahoppen]

We deliberately only included the parser library in the release tags and not the release/5.6 branch because that defines away any issues with the SwiftSyntax sources changing without updating the URL of the parser library (which won’t even be available at the point that the SwiftSyntax sources get updated). So the idea is that release/5.6 represents the latest development state for a specific Swift release and we cut release tags that vendor the parser library from that branch.

[keith]

Thanks for the context! That makes sense, but I think it makes it a bit harder to understand if your build is behaving as you expect if you also need to include other patches like #372 in the meantime. We specifically hit this while testing the combo of the branch vs tag + https://github.com/keith/StaticInternalSwiftSyntaxParser

Is there high risk of the binary needing to be updated on this same branch? In the past we've often used the same version of that binary even if we update SwiftSyntax at least for the same release branch

[dduan]

@ahoppen Does it make sense to introduce an environment variable to turn this target on and off? It'll help users who want to use an alternative like https://github.com/keith/StaticInternalSwiftSyntaxParser and, potentially, make this extra release step not necessary.

[ahoppen]

Thanks for the context! That makes sense, but I think it makes it a bit harder to understand if your build is behaving as you expect if you also need to include other patches like #372 in the meantime. We specifically hit this while testing the combo of the branch vs tag + https://github.com/keith/StaticInternalSwiftSyntaxParser

I would hope that this is a a problem that only happened once because we switched to the new tagging scheme that vendors the parser library. My hope is that in the future, adopters will try using the snapshot releases of SwiftSyntax (like 0.50600.0-SNAPSHOT-2022-01-24) and discover such issues earlier on in the development cycle where making fixes is simpler and less disruptive. Also, if issues like these keep occurring, we need to re-evaluate our testing of SwiftSyntax releases.

Is there high risk of the binary needing to be updated on this same branch? In the past we've often used the same version of that binary even if we update SwiftSyntax at least for the same release branch

I would say the risk is low but it’s definitely not 0%. For sake of argument assume that we find a major flaw in syntax parsing in 5.6 that requires changing the parser library and that fix makes it into a Swift 5.6.1 release. In that case, when updating the SwiftSyntax sources to match the new parser library, we need to either

• Remove the binary dependency on the now-outdated parser library when updating the sources — this is very easy to forget and it regresses the quality of the release/5.6 branch because it now stopped venturing the parser library
• Live with the fact that the parser library is outdated for a couple of days until we have a new snapshot toolchain from which we can publish the parser library – this also drastically regresses the release/5.6 quality in unexpected ways, also we need to remember to update the parser library when a new one is available, which again is easy to forget

To avoid these unexpected regressions and pitfalls, I decided that it’s clearer if release/5.6 just never contains the parser library and only the release tags do. The real issue is that we need to solve a chicken-and-egg problem for any moving branch: We need to update the SwiftSyntax sources to produce a new toolchain but we need to have a new toolchain so we can update the binary dependency to match the updated SwiftSyntax sources.

If you have a better idea of handling things, I would be very interested in discussing them.

@ahoppen Does it make sense to introduce an environment variable to turn this target on and off? It'll help users who want to use an alternative like https://github.com/keith/StaticInternalSwiftSyntaxParser and, potentially, make this extra release step not necessary.

We can certainly consider that. But if I understand correctly the purpose of https://github.com/keith/StaticInternalSwiftSyntaxParser is exactly what is now served by SwiftSyntax itself, so it shouldn’t be necessary anymore, right?

---

Unrelated to your comments but you are probably the right audience: If you would like me to create a prerelease of SwiftSyntax (including the parser library) for any development branch, just ping me and I’ll do it. We don’t currently have a fixed prerelease schedule, the current plan is to do it on a per-need basis and whenever we feel that enough changes have accumulated to make a new prerelease useful.

[keith]

Remove the binary dependency on the now-outdated parser library when updating the sources — this is very easy to forget and it regresses the quality of the release/5.6 branch because it now stopped venturing the parser library

My assumption is that this wouldn't be easy to forget if it was motivated by a bug specifically found in the parser, and it would be easier to forget to add an extra commit off the branch for the tag, but I'm not sure what your release process looks like and if you're scripting that piece.

If you have a better idea of handling things, I would be very interested in discussing them.

Slightly related: I think now that SwiftSyntax is entirely ignoring Xcode's version of this library, we should revisit shipping it statically in the toolchain for this use case (and still ship dynamically for other tools if needed). My assumption as the reason it was shipped dynamically before (besides the impact on binary size if many tools depended on it) was that it was attempting to always match your version of Swift with Xcode, instead of what SwiftSyntax was compiled with, but now that it's vendored it seems like that's no longer the case.

We can certainly consider that. But if I understand correctly the purpose of keith/StaticInternalSwiftSyntaxParser is exactly what is now served by SwiftSyntax itself, so it shouldn’t be necessary anymore, right?

My version produces a smaller binary because we get the dead stripping benefits of static libraries, it looks like in swiftlint's case we only save ~3mbs, but that's assuming you thin the multiarch binary and strip it (and invalidate the code signature from the dylib). For reference if you don't strip the dylib you end up with ~100mbs of artifacts vs ~40mbs with the static lib. More importantly it's easier to redistribute a single binary versus having to distribute the binary with the dylib alongside it and potentially manipulate rpaths for your distribution environment.

[ahoppen]

My assumption is that this wouldn't be easy to forget if it was motivated by a bug specifically found in the parser, and it would be easier to forget to add an extra commit off the branch for the tag, but I'm not sure what your release process looks like and if you're scripting that piece.

My concern is still that people start to rely on release/5.6 shipping with a binary parser library and we need to break that assumption for a couple of days if we update the SwiftSyntax sources, which IMHO could be very surprising.

Re: Static Linking

Linking statically against the parser library definitely makes sense. Thanks for suggesting it 👍 I will look into it, either as a follow-up release of 0.50600.1 or for the next major release. I’m fairly busy at the moment though and I might not get to it in the next month.