invenia / Memento.jl

A flexible logging library for Julia
https://invenia.github.io/Memento.jl/latest
Other
87 stars 14 forks source link

Dependency on TimeZones.jl breaks build #104

Closed essenciary closed 4 years ago

essenciary commented 6 years ago

Is it really necessary to have this dependency? TimeZones.jl consistently breaks build on Ubuntu and I had reports of happening the same on Windows in some cases.

  Building TimeZones → `~/.julia/packages/TimeZones/wytr8/deps/build.log`
┌ Error: Error building `TimeZones`: 
│ [ Info: Downloading 2018e tzdata
│ ERROR: LoadError: IOError: could not spawn `curl -g -L -f -o /home/parallels/.julia/packages/TimeZones/wytr8/deps/tzarchive/tzdata2018e.tar.gz https://www.iana.org/time-zones/repository/releases/tzdata2018e.tar.gz`: no such file or directory (ENOENT)
│ Stacktrace:
│  [1] _jl_spawn(::String, ::Array{String,1}, ::Cmd, ::Tuple{RawFD,RawFD,RawFD}) at ./process.jl:367
│  [2] (::getfield(Base, Symbol("##495#496")){Cmd})(::Tuple{RawFD,RawFD,RawFD}) at ./process.jl:509
│  [3] setup_stdio(::getfield(Base, Symbol("##495#496")){Cmd}, ::Tuple{RawFD,RawFD,RawFD}) at ./process.jl:490
│  [4] #_spawn#494(::Nothing, ::Function, ::Cmd, ::Tuple{RawFD,RawFD,RawFD}) at ./process.jl:508
│  [5] _spawn at ./process.jl:504 [inlined]
│  [6] #run#505(::Bool, ::Function, ::Cmd) at ./process.jl:652
│  [7] run at ./process.jl:651 [inlined]
│  [8] download(::String, ::String) at ./download.jl:48
│  [9] tzdata_download(::String, ::String) at /home/parallels/.julia/packages/TimeZones/wytr8/src/tzdata/download.jl:90
│  [10] macro expansion at ./logging.jl:310 [inlined]
│  [11] #build#32(::Bool, ::Function, ::String, ::Array{String,1}, ::String, ::String, ::String) at /home/parallels/.julia/packages/TimeZones/wytr8/src/tzdata/build.jl:28
│  [12] #build at ./none:0 [inlined]
│  [13] build(::String, ::Array{String,1}) at /home/parallels/.julia/packages/TimeZones/wytr8/src/tzdata/build.jl:72
│  [14] #build#5(::Bool, ::Function, ::String, ::Array{String,1}) at /home/parallels/.julia/packages/TimeZones/wytr8/src/TimeZones.jl:116
│  [15] build at /home/parallels/.julia/packages/TimeZones/wytr8/src/TimeZones.jl:116 [inlined] (repeats 2 times)
│  [16] top-level scope at none:0
│  [17] include at ./boot.jl:317 [inlined]
│  [18] include_relative(::Module, ::String) at ./loading.jl:1038
│  [19] include(::Module, ::String) at ./sysimg.jl:29
│  [20] include(::String) at ./client.jl:388
│  [21] top-level scope at none:0
│ in expression starting at /home/parallels/.julia/packages/TimeZones/wytr8/deps/build.jl:6
└ @ Pkg.Operations /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.0/Pkg/src/Operations.jl:1068
rofinn commented 6 years ago

Hi,

Thanks for submitting this issue. Could you provide some extra details as it's hard to tell whether this is a julia, timezones or system configuration issue?

  1. What version of Ubuntu is this?
  2. Can this be reproduced in a publicly available linux vm or docker image?
  3. What's the output of Sys.which("curl") on your system? My initial thought is that the download command should include the full path to the curl binary, but the Sys.which("curl") call happening here should be finding curl on the PATH.
  4. Does the download function work for other urls or paths?

Regarding whether Memento should depend on TimeZones is probably be a larger conversation. Apart from downloading and building the tz database I'd consider it a pretty light dependency and I like the idea of having timezone aware logs by default.

Regards

essenciary commented 6 years ago

Hi,

Thanks for getting back to me.

Actually, I managed to dive a bit deeper into this. So we're talking about Ubuntu 16.04 -- a fresh install through Parallels Desktop (running on macOS).

The issue is that curl does not come bundled with the OS. Running the

curl -g -L -f -o /home/parallels/.julia/packages/TimeZones/wytr8/deps/tzarchive/tzdata2018e.tar.gz https://www.iana.org/time-zones/repository/releases/tzdata2018e.tar.gz

command in the terminal errored saying that curl is not installed.

Running

sudo apt-get curl

did fix the problem.

On the other hand wget is distributed with the os. So maybe you could use that? Or have it as a fallback?

Or maybe better, HTTP -- I presume it handles the various OS issues?


Update 1 - ugh, why am I putting all this here, it should be in TimeZones.jl :)

rofinn commented 6 years ago

This seems like an issue with the implementation of download in base julia, so TimeZones.jl wouldn't be the place for it either. The download function should fallback to wget if curl isn't installed, so I'm wondering if Sys.which("curl") isn't returning nothing on your system. Once we better understand the issue we can file an issue/PR against base julia.

omus commented 6 years ago

Cross reference: https://github.com/JuliaTime/TimeZones.jl/issues/153

rofinn commented 6 years ago

Oh, looks like this is already fixed in master, and is currently sitting in the backports branch for 1.0.1.

https://github.com/JuliaLang/julia/commit/d6e43e25ff7ca57ff85342243fd072e51ae303d5

essenciary commented 5 years ago

@rofinn @omus Thanks for looking into it -- that's good to know. Hopefully, we'll see the fix soon, as it crashes the Genie installation and it's a pain in the cheeks.

essenciary commented 5 years ago

Another issue on Windows -- possibly due to firewall restrictions: https://github.com/essenciary/Genie.jl/issues/59

In my opinion, this kind of coupling is not doing Memento any service...

rofinn commented 5 years ago

Okay, so AFAICT there are 3 issues here w/ 1 that relates to Memento directly:

  1. download in julia 1.0 is broken and we're currently waiting on 1.0.1 to be released. https://github.com/JuliaLang/julia/pull/28764
  2. TimeZones.localzone errors when machines are using a deprecated timezones (e.g., America/Indianapolis). https://github.com/JuliaTime/TimeZones.jl/issues/154
  3. Julia doesn't have a good solution for optional dependencies (discussed in multiple places over the last few years), so we may want to have a minimal base package that doesn't have any external dependencies. https://github.com/invenia/Memento.jl/issues/105
omus commented 4 years ago

With https://github.com/JuliaTime/TimeZones.jl/issues/154 being fixed and Julia 1.0.1 being released long ago I think it's safe to close this issue.