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

tzdata: Use hard- instead of symbol-linking #31560

Closed
wants to merge 2 commits into from

Conversation

ThomasMader
Copy link
Contributor

@ThomasMader ThomasMader commented Nov 12, 2017

Motivation for this change

Upstream software parses ${tzdata}/share/zoneinfo files.
Because of the symbolic posix link pointing to the local dir, the upstream software parses the contents of the zoneinfo directory multiple times because it follows symlinks and an error occurs because of this.
This PR changes the posix symlink to a hardlink to have a normal directory for posix like Fedora does.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

@ThomasMader ThomasMader changed the title tzdata: Keep the standard directories and links tzdata: Use hard- instead of symbol-linking Nov 13, 2017
@joachifm
Copy link
Contributor

I notice that 1f2228c changed from hard to soft links citing NAR issues. Is that relevant here?

@vcunat
Copy link
Member

vcunat commented Nov 22, 2017

IIRC NARs don't preserve hardlinks, i.e. it's effectively as if you did a plain copy to most users, and the files being hardlinked will depend primarily on user's "optimising" their nix store. I don't know about any other issues.

@orivej
Copy link
Contributor

orivej commented Nov 22, 2017

Because of the symbolic posix link pointing to the local dir, the upstream software parses the contents of the zoneinfo directory multiple times because it follows symlinks and an error occurs because of this.

What software? What error?

@ThomasMader
Copy link
Contributor Author

What software? What error?

The standard library of the D programming language called Phobos.
I already patched it to not follow symlinks (see ThomasMader@6e15600#diff-9da5b94b5c73489302d04c387fbbf0c9R91) but wanted to make sure that tzdata has a standard layout before going upstream.
In Commit 1f2228c is mentioned though that other distributions are using symlinking too.
If that is true this PR can be closed.

@orivej
Copy link
Contributor

orivej commented Nov 22, 2017

Ubuntu is using symlinking, but without loops. This is better: e75d458

@orivej
Copy link
Contributor

orivej commented Nov 23, 2017

I've pushed my fix to staging. GitHub should close this issue when it reaches master.

@FRidh FRidh closed this in 9d2bd32 Nov 25, 2017
@orivej
Copy link
Contributor

orivej commented Nov 25, 2017

@FRidh Thank you, I was considering doing this myself.

@FRidh
Copy link
Member

FRidh commented Nov 25, 2017

@orivej that was github; I merged staging into master.

@orivej
Copy link
Contributor

orivej commented Nov 25, 2017

@FRidh My bad, I did not expect staging to be merged to master so soon. But please, configure git with git config --global merge.ff false (and git config --global pull.rebase true), or at least use git merge --no-ff when merging staging into master, or it messes up the first-parent history and the history tree of the repository. (This effect was described here and here.) I have almost recovered the proper history in fbbda41.

@ThomasMader ThomasMader deleted the tzdata branch December 6, 2017 23:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants