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

Users need to download all timezones when using create_app from PackageCompiler #300

Closed
benhamad opened this issue Oct 8, 2020 · 8 comments

Comments

@benhamad
Copy link

benhamad commented Oct 8, 2020

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
....
@omus
Copy link
Member

omus commented Oct 9, 2020

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
Copy link
Member

omus commented Oct 10, 2020

@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?

@staticfloat
Copy link
Contributor

@KristofferC is this intended behavior for PackageCompiler?

@KristofferC
Copy link
Contributor

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.

@staticfloat
Copy link
Contributor

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?

@KristofferC
Copy link
Contributor

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.

@benhamad
Copy link
Author

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

@omus
Copy link
Member

omus commented Aug 22, 2023

In PR #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 omus closed this as completed Aug 22, 2023
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

No branches or pull requests

4 participants