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

Relocatability no longer given due to link to TZJData.ARTIFACT_DIR and storing path in const string reference #467

Open
divbyzerofordummies opened this issue Jul 9, 2024 · 4 comments · May be fixed by #479

Comments

@divbyzerofordummies
Copy link

We have encountered a relocatability issue when creating a multi-stage docker container where we copy an app created by PackageCompiler to a clean image after having built it.

It now looks for the artifact in the old directory which was valid at build time and is no longer available in the final container.

The hash of the artifact is 520ce3f83be7fbb002cca87993a1b1f71fe10912 and I have verified that this is the hash of the artifact created when imporing TZJData.

Looking for this problem I have found a post with a similar problem with a solution in the following commit.

The problem seems to be the definition of a constant in TZJData.jl:

const ARTIFACT_DIR = artifact"tzjdata" 

which is referenced in TimeZones.jl (again, with the const keyword, but this time on the reference, so the value is very much mutable):

const _COMPILED_DIR = Ref{String}(TZJData.ARTIFACT_DIR)

According to the comment in the post mentioned above, "Is it because the const makes the path fixed at compilation, whereas we need it to load and find the artifact each time?", this seems to be a bad idea when trying to keep a package portable.

The solution seems to make both const definitions functions instead, i.e., in TZJData.jl:

function artifact_dir()
    return artifact"tzjdata" 
end

(not sure about the necessity to put this into a function in the first place), and in TimeZones.jl:

function compiled_dir()
    return TZJData.artifact_dir()
end

BUT I am not really sure how this works when building with a different tzdata version: In TimeZones.jl, it says:

function _build()
    desired_version = TZData.tzdata_version()
    if desired_version != TZJData.TZDATA_VERSION
        _COMPILED_DIR[] = TZData.build(desired_version, _scratch_dir())
    end

    return nothing
end

I have absolutely no clue how/whether this changed content of the referenced string ends up in my compiled app... and if it does (which I assume) how portability could still be ensured, because now it is definitely an incorrect path on my final docker image.

But I guess for the vanilla user not requesting a special version of tzdata, my solution would work, but I would need an extra string reference to the new compile dir if the user chooses a different version. That would be quite ugly and still not portable for special tzdata requirements...

const _CUSTOM_TZDATA_DIR = Ref{String}("")
function compiled_dir()
    return if isempty(_CUSTOM_TZDATA_DIR[])
        TZJData.artifact_dir()
    else
        _CUSTOM_TZDATA_DIR[]
    end
end

I assume / hope more experienced Julia programmers have better ideas?

@omus
Copy link
Member

omus commented Sep 4, 2024

Thanks for bringing this up. An alternate solution could be to utilize __init__ functions to populate Ref constants. The benefit of this approach is that it lets us control when we perform the artifact check.

Something we want to avoid is having the artifacts downloaded at runtime within a Docker container. Ideally, precompilation and artifacts are installed during the PackageCompiler process so we aren't suddenly trying to install artifacts during the container runtime. As you mentioned the artifact hash remains consistent in your two different container environments so maybe an alternate solution would be to update TZJData.ARTIFACT_DIR to be a path relative to the Base.DEPOT_PATH. This should allow for good package loading performance and improve portability.

If you want to create PRs with such approaches you should be able to validate they work in your specific setup (use pkg> add TimeZones#branch-name) or alternatively if you could provide a MWE of your setup I can work on the issue.

@omus
Copy link
Member

omus commented Sep 4, 2024

We probably do want to calculate the artifact location at runtime so that overrides are applied. I'll need to think about this more yet.

@t-bltg
Copy link

t-bltg commented Oct 4, 2024

I am also encountering relocability issues having a relocated artifacts directory, and the current approach fails precompilation.

Has there been updates on this topic ?

@DilumAluthge
Copy link
Contributor

As you mentioned the artifact hash remains consistent in your two different container environments so maybe an alternate solution would be to update TZJData.ARTIFACT_DIR to be a path relative to the Base.DEPOT_PATH.

Maybe the RelocPath type (from JuliaLang/julia#56053) could be useful here?

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

Successfully merging a pull request may close this issue.

4 participants