Skip to content

[Diag] Create a serialized format for Localized Diagnostics #33022

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 8 commits into from
Aug 6, 2020
Merged

[Diag] Create a serialized format for Localized Diagnostics #33022

merged 8 commits into from
Aug 6, 2020

Conversation

HassanElDesouky
Copy link
Contributor

Introduces a serialized file format and create a tool for converting the diagnostic file from .yaml to .db

@HassanElDesouky
Copy link
Contributor Author

cc @xedin

@xedin xedin self-requested a review July 21, 2020 00:34
xedin
xedin requested changes Jul 22, 2020
View reviewed changes
Copy link
Contributor

@xedin xedin left a comment

Choose a reason for hiding this comment

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

Direction looks good! I have left some comments in line.

xedin
xedin reviewed Jul 22, 2020
View reviewed changes
xedin
xedin reviewed Jul 22, 2020
View reviewed changes
HassanElDesouky
HassanElDesouky commented Jul 23, 2020
View reviewed changes
HassanElDesouky
HassanElDesouky commented Jul 23, 2020
View reviewed changes
HassanElDesouky
HassanElDesouky commented Jul 23, 2020
View reviewed changes
@HassanElDesouky HassanElDesouky marked this pull request as ready for review July 23, 2020 19:52
@HassanElDesouky HassanElDesouky requested a review from xedin July 23, 2020 19:52
xedin
xedin requested changes Jul 24, 2020
View reviewed changes
Copy link
Contributor

@xedin xedin left a comment

Choose a reason for hiding this comment

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

Getting closer! I've left some more comments inline.

@HassanElDesouky HassanElDesouky force-pushed the localization-serialization branch 2 times, most recently from 09c6450 to 677b09d Compare July 27, 2020 10:24
xedin
xedin reviewed Jul 27, 2020
View reviewed changes
@HassanElDesouky HassanElDesouky force-pushed the localization-serialization branch 3 times, most recently from 64393f2 to 3c8fdb6 Compare July 29, 2020 11:53
@xedin xedin force-pushed the localization-serialization branch from 3c8fdb6 to bc4ff41 Compare July 30, 2020 19:05
@xedin
Copy link
Contributor

xedin commented Jul 30, 2020

@swift-ci please build toolchain

@swift-ci
Copy link
Contributor

macOS Toolchain
Download Toolchain
Git Sha - bc4ff41da4bed49605c94a2b63a32e78b0b7b672

Install command
tar -zxf swift-PR-33022-587-osx.tar.gz --directory ~/

@xedin
Copy link
Contributor

xedin commented Jul 31, 2020

@swift-ci please build toolchain

1 similar comment
@owenv
Copy link
Contributor

owenv commented Aug 1, 2020

@swift-ci please build toolchain

@xedin
Copy link
Contributor

xedin commented Aug 3, 2020

@swift-ci please build toolchain macOS platform

@HassanElDesouky
Copy link
Contributor Author

HassanElDesouky commented Aug 3, 2020 •
edited
Loading

@xedin why did the build fail? https://ci.swift.org/job/swift-PR-toolchain-osx/lastBuild/console

@xedin
Copy link
Contributor

xedin commented Aug 3, 2020

CI issue, let's try again.

@xedin
Copy link
Contributor

xedin commented Aug 3, 2020

@swift-ci please build toolchain macOS platform

1 similar comment
@xedin
Copy link
Contributor

xedin commented Aug 3, 2020

@swift-ci please build toolchain macOS platform

@xedin
Copy link
Contributor

xedin commented Aug 3, 2020

@shahmishal Looks like there is a problem with github auth on one of the machines.

@shahmishal
Copy link
Member

Should be fixed, please try again

@xedin
Copy link
Contributor

xedin commented Aug 3, 2020

@swift-ci please build toolchain macOS platform

@xedin
Copy link
Contributor

xedin commented Aug 3, 2020

@shahmishal Thanks, looks like it worked!

@swift-ci
Copy link
Contributor

swift-ci commented Aug 3, 2020

macOS Toolchain
Download Toolchain
Git Sha - 87d2d15f0cfa469c570e6e54d9ee4c35d54de7a9

Install command
tar -zxf swift-PR-33022-593-osx.tar.gz --directory ~/

@xedin
Copy link
Contributor

xedin commented Aug 3, 2020

Great! Looks like we got .db in the toolchain now, I can make it so everything is placed into the share/swift/diagnostics and copied from there

@xedin xedin force-pushed the localization-serialization branch from 87d2d15 to d0f2b73 Compare August 4, 2020 23:27
@xedin
Copy link
Contributor

xedin commented Aug 4, 2020

@swift-ci please clean build toolchain

@xedin
Copy link
Contributor

xedin commented Aug 4, 2020

@swift-ci please build toolchain

@xedin xedin force-pushed the localization-serialization branch from d0f2b73 to 6a30f18 Compare August 5, 2020 21:49
@xedin
Copy link
Contributor

xedin commented Aug 5, 2020

@swift-ci please build toolchain

HassanElDesouky and others added 8 commits August 5, 2020 15:52
@HassanElDesouky @xedin
[Locale] Serialize YAML to an OnDiskHashTable format and create a too…
a84cf7f 
…l for serialization
@xedin
[Localization] Simplify implementation of iterator over YAML
8ee4e93
@xedin
[Localization] Serialization writer should own the inserted data
8a1b620
@xedin
[Localization] Adjust types associated with reader/writer info
8d8da1b 
Add more usages of `offset_type` where appropriate and change
buffer type to `const unsigned char *`.
@xedin
[Localization] Adjust cmake to include serialized diagnostics in tool…
0103b79 
…chain

Building of the diagnostic database to be done in three steps:

- Copy YAMLs from <source>/localization/diagnostics to <build>/share/swift/diagnostics
- Serialize English into <build>/share/swift/diagnostics
- Install everything (YAML and `.db`) from <build>/share/swift/diagnostics
  to share/swift/diagnostics in toolchain
@xedin
[Localization] NFC: Include all of the headers based on the types used
67dbef5
@xedin
[Localization] Switch serialization format to use DiagID as a key
3a4d156
@xedin
[Localization] Make return type of the serialization main cross platform
5e13a53
@xedin xedin force-pushed the localization-serialization branch from 6a30f18 to 5e13a53 Compare August 6, 2020 00:08
@xedin
Copy link
Contributor

xedin commented Aug 6, 2020

@swift-ci please test

@xedin
Copy link
Contributor

xedin commented Aug 6, 2020

@swift-ci please build toolchain

@xedin
Copy link
Contributor

xedin commented Aug 6, 2020

@swift-ci please asan test

@swiftlang swiftlang deleted a comment from swift-ci Aug 6, 2020
@swiftlang swiftlang deleted a comment from swift-ci Aug 6, 2020
@xedin
Copy link
Contributor

xedin commented Aug 6, 2020

@swift-ci please ASAN test

@xedin
Copy link
Contributor

xedin commented Aug 6, 2020

@shahmishal Both toolchain builds were successful but none updated their state :/

@xedin
Copy link
Contributor

xedin commented Aug 6, 2020

Ok, both toolchains look good now. Let's wait for the tests to run and we'll be good to go with this.

@xedin
Copy link
Contributor

xedin commented Aug 6, 2020

ASAN failure is in GSB and is unrelated to these changes.

@xedin xedin merged commit 2b62570 into swiftlang:master Aug 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@xedin xedin xedin approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@HassanElDesouky @xedin @swift-ci @owenv @shahmishal