Environment dependencies for stdlib specs

[straight-shoota]

The spec suite for stdlib APIs has some dependencies on the system environment. The specs make assumptions about the file system and available programs. They're required for the specs to pass. Those dependencies might not be available in all cases. And they're quite hidden, so you'll likely only learn about it when a spec fails due to missing prerequisites.

The nix package for Crystal applies a number of changes to make the spec suite work in a nix environment:

https://github.com/NixOS/nixpkgs/blob/95968fcd7b341d65c9e7202850d52e563f6591de/pkgs/development/compilers/crystal/default.nix#L130-L149

Now of course, nix is very special in these regards. But it's definitely a good reminder that some assumption may not fit for all systems.

Failing specs are bad. Patches like the ones in the nix package to make specs work, are also not great. They introduce a dependency on the spec code which can break at any moment.

I don't think it should be a package maintainer's responsibility to fix these specs to work in an (from the author's perspective) unexpected (but nevertheless valid) environment.

One class of problems are dependencies on file paths. Those are easy to resolve. Instead of using fixed paths, we can just use paths that are explicitly known (form the data directory) or create a temporary files/directories as necessary for the particular test environment. I have a PR ready for that at #12776

Another problem class are executables. They are mainly used for process_spec. There is already some generalization to account for differences beween unix and windows systems:

https://github.com/crystal-lang/crystal/blob/5397f1e6fc82f014076a7806dac583736bfee05a/spec/std/process_spec.cr#L7-L53

Apart from that, there are not that many uses:

• shell_command is only used in specs as shell_command("echo hello") (once with additional output stream redirection`).
• system_spec uses hostname and getconf programs (the latter doesn't seem to be problematic in a nix environment).

Apart from hostname and getconf the program features necessary for the test are very simple and just cover basic process features such as return types, streams and environment variables. It should be very easy to implement these necessary features in a Crystal program and use that instead of depending on system programs. This would be very portable and make it easier to write specs for different platforms.

WDYT about that idea? Are there alternative suggestions?

/cc @GeopJr @bcardiff

[bcardiff]

Another dependency in the nix package is:

substituteInPlace src/crystal/system/unix/time.cr \
  --replace /usr/share/zoneinfo ${tzdata}/share/zoneinfo

If we would like to offer that substitution directly the best way would be to introduce a CRYSTAL_CONFIG_ZONEINFO that when set would define the first (or only) value in Crystal::System::Time::ZONE_SOURCES.

But doing that would mean that at least by default any compiled program from crystal should have the same tzdata dependecy, or the directory will not exist.

Maybe instead of the substituteInPlace we should use the ZONEINFO env var and suggest crystal nix users to do it also.

What should be the preferred approach?

[straight-shoota]

IMO this doesn't really need much special handling for this particular case. /usr/share/zoneinfo is the standard location for the time zone database. Nix uses some non-standard path. It's not a big deal for nix to explicitly patch the location in the source. The main reason for error would be if the location of this string changes. But that's not very likely.

That said I wouldn't object to having a compile time configuration option. CRYSTAL_CONFIG_ZONEINFO should work well for this

I don't understand what you mean by this limitation:

But doing that would mean that at least by default any compiled program from crystal should have the same tzdata dependecy, or the directory will not exist.

[bcardiff]

But doing that would mean that at least by default any compiled program from crystal should have the same tzdata dependency, or the directory will not exist.

I might be wrong here, I still don't know enough nix. If the crystal compiler at nix depends on tzdata as build input we are guaranteeing that the users of the compiler will have that package. But what happens with the programs built from this compiler? as long as they are only used in the compiled environment the referenced tzdata will be there. Otherwise I don't see how that dependency would be propagated. If we hardcode the path via a substitution we will need to force the same package to be used when building. What I imagine would work better is to make buildCrystalPackage function add tzdata package as a dependency and embed / define it's path for the generated program. So this actually push the idea of having a [CRYSTAL_]CONFIG_ZONEINFO that could embed where to do the lookup (not only for the compiler, hence maybe it should not be prefixed with CRYSTAL_ (?))

[GeopJr]

FWIW, the nix package for go prepends "@tzdata@/share/zoneinfo/"

https://github.com/NixOS/nixpkgs/blob/8bda09a85d8d45616ce2800d8aa63afa8f45ade1/pkgs/development/compilers/go/tzdata-1.19.patch#L9

[straight-shoota]

@bcardiff Okay, gotcha. Yeah that can be a problem.

@GeopJr Prepending the nix path sounds like a good idea. I would've suggested that as well. Patching that into the source code sounds fine. I don't think we need a config variable for that.