Conversation
|
What are we seeing size-wise with this change? |
| <RemoveDir Directories="$(TimeZoneDataSrcDir)" /> | ||
| <DownloadFile SourceUrl="https://data.iana.org/time-zones/tzdb-$(TimeZoneDataVersion)/%(Area.Identity)" | ||
| DestinationFolder="$(TimeZoneDataIntermediateDir)" /> | ||
| <Exec Command="wget -r -nH --cut-dirs=2 --no-parent --reject='index.html*' -P $(TimeZoneDataIntermediateDir) https://data.iana.org/time-zones/tzdb-2021a/" /> |
There was a problem hiding this comment.
| <Exec Command="wget -r -nH --cut-dirs=2 --no-parent --reject='index.html*' -P $(TimeZoneDataIntermediateDir) https://data.iana.org/time-zones/tzdb-2021a/" /> | |
| <Exec Command="wget -r -nH --cut-dirs=2 --no-parent --reject='index.html*' -P $(TimeZoneDataIntermediateDir) https://data.iana.org/time-zones/tzdb-$(TimeZoneDataVersion)/" /> |
| <DownloadFile SourceUrl="https://data.iana.org/time-zones/tzdb-$(TimeZoneDataVersion)/%(Area.Identity)" | ||
| DestinationFolder="$(TimeZoneDataIntermediateDir)" /> | ||
| <Exec Command="wget -r -nH --cut-dirs=2 --no-parent --reject='index.html*' -P $(TimeZoneDataIntermediateDir) https://data.iana.org/time-zones/tzdb-2021a/" /> | ||
| <Exec Command="touch README" WorkingDirectory="$(TimeZoneDataIntermediateDir)"/> |
There was a problem hiding this comment.
We need to download the entire tzdb directory instead of just specific files. make zic doesn't work without a README, which is not (weirdly) included in the directory itself.
There was a problem hiding this comment.
...huh, is there any chance of PRing a fix for that upstream? This is fine as a workaround, but we should look into why that is.
There was a problem hiding this comment.
It's possible we should switch to using the full tar.lz that contains everything no.
Early results are good, we're a little worried that the runtime tests aren't exhaustive enough. |
Can you elaborate on this? Specifically what concerns do you have? In my experience, the TimeZoneInfo tests are pretty decent. |
We have absolutely run into several problems by targeting the tests too narrowly (for size savings) and we're just trying to avoid that happening again. |
lewing
left a comment
There was a problem hiding this comment.
This changes the build targets that aren't actually being run as part of CI, but it doesn't update the data that is generated and consumed on the dotnet/runtime side
|
So the binary diff is somewhere between 20-70% based on what GitHub displays. That's great to see. |
There was a problem hiding this comment.
Looks good now. @ViktorHofer the final compressed size comes in at about 43796 bytes which is solid savings.
Reduce size of timezone data, by compiling zic from IANA and using
-b slimfor tz compilation. Fixes: dotnet/runtime#48061