TimeZones.jl icon indicating copy to clipboard operation
TimeZones.jl copied to clipboard

Users need to download all timezones when using create_app from PackageCompiler

Open benhamad opened this issue 4 years ago • 7 comments

create_app from package compiler download all the artifacts from all the decencies of the package to be built. This is causes our CI to download all timezons in Artifacts.toml!

Downloading artifact: tzdata2007f
Downloading artifact: tzdata2019b
Downloading artifact: tzdata2012j
Downloading artifact: tzdata2011m
Downloading artifact: tzdata1997b
....

benhamad avatar Oct 08 '20 22:10 benhamad

Thanks for filing this issue. That's definitely a suboptimal behavior. I'll look into possible solutions without just rewriting the Artifacts.toml to contain the current release.

omus avatar Oct 09 '20 01:10 omus

@staticfloat your opinion here would be appreciated.

I setup the Arifacts.toml to allow any tzdata file to be used. In reality only the latest tzdata version is needed and typically an earlier version used in testing. All the artifacts were setup as lazy to only download what is required.

PackageCompiler is fetching all artifacts, even those marked as lazy, for relocation purposes. Should this behavior be changed, modified with some additional artifact flag, or should I restrict the artifact list to the bare minimum? If the answer is reducing the artifact list is there another good way to handle retrieving other tzdata through the artifact system?

omus avatar Oct 10 '20 02:10 omus

@kristofferc is this intended behavior for PackageCompiler?

staticfloat avatar Oct 11 '20 16:10 staticfloat

Well, it depends on what you mean with "intended". The code in PackageCompiler is written to do this, yes, but whether this is a good idea if there should be an option to chose the inclusion of lazy_download, etc., is unclear to me.

KristofferC avatar Oct 11 '20 17:10 KristofferC

Yeah, I meant eager downloading of lazy artifacts. I think this should probably be an option, since there are legitimate usecases where you really just want to ensure that the bundled application never downloads anything, but then there are obvious cases like this where it is not meaningful to do this.

One way to work around this is to break PackageCompiler's autodetection by storing the artifacts in SomeOtherName.toml, but then you'd need to manually load the .toml file at compile time, then index into it, rather than use the @artifact_str macro.

That would look something like the following:

using Artifacts
pkg_uuid = Base.UUID("my_uuid") # This isn't needed for most people, but if someone wants to use an Overrides.toml on you, you need to tell the loading code what your UUID is.
artifacts_dict = load_artifacts_toml(joinpath(@__DIR__, "Artifacts.toml"); pkg_uuid)

function lookup_artifact_path(name::String)
    # At runtime, index into `artifacts_dict` using the current runtime platform.
    # note that the third argument is just a path used in error messages, so it's
    # okay if it's stale.
    meta = artifact_meta(name, artifacts_dict, joinpath(@__DIR__, "Artifacts.toml"))
    return artifact_path(meta["git-tree-sha1"])
end

So then you'd just use lookup_artifact_path(name) instead of @artifact_str(name).

Another way to work around this is to be able to signal to PackageCompiler somehow that it really should consider something a lazy artifact. Perhaps we should develop a convention of storing a list of names in .pkg/packagecompiler_lazy_artifacts.txt, and any names listed in that file (if they correspond to a lazy artifact) don't get downloaded, or somesuch?

staticfloat avatar Oct 11 '20 17:10 staticfloat

Perhaps better to make a small PR to PackageCompiler that has an include_lazy_artifacts option instead of messing around too much with the package based on (perhaps) bad defaults in PackageCompiler.

KristofferC avatar Oct 11 '20 17:10 KristofferC

iana.org started to rate limit our builds due to this issue.

Downloading artifact: tzdata96c
curl: (7) Failed to connect to data.iana.org port 443: Connection refused

benhamad avatar Oct 19 '20 07:10 benhamad

In PR https://github.com/JuliaTime/TimeZones.jl/pull/441 all of the lazy tzdata artifacts were removed in favor of using precompiled time zones provided by TZJData.jl non-lazy artifacts. The end result is that when using PackageCompiler it doesn't matter if you use include_lazy=true or not you'll just download the a single tzdata artifact.

Included in TimeZones.jl release 1.12.

omus avatar Aug 22 '23 04:08 omus