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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

proj-data: init at 1.16.0 #280062

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
73 changes: 73 additions & 0 deletions pkgs/by-name/pr/proj-data/package.nix
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
{ lib
, stdenv
, callPackage
, fetchFromGitHub

# proj-data grid directories to install (ex.: "nz_linz").
# By default, no grids are installed due to the large size (size of all proj-
# data grids is nearly 1GB and will grow over the time).
, gridPackages ? [ ]
}:

stdenv.mkDerivation (finalAttrs: {
pname = "proj-data";
version = "1.16.0";

src = fetchFromGitHub {
owner = "OSGeo";
repo = "PROJ-data";
rev = finalAttrs.version;
hash = "sha256-/EgDeWy2+KdcI4DLsRfWi5OWcGwO3AieEJQ5Zh+wdYE=";
};

installPhase = ''
runHook preInstall
shopt -s extglob

mkdir -p $out
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
mkdir -p $out
mkdir -p "$out"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the comment about store path variables quoting above.

cp README.DATA $out/README-PROJ-DATA.md
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
cp README.DATA $out/README-PROJ-DATA.md
cp README.DATA "$out"/README-PROJ-DATA.md

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the comment about store path variables quoting above.


for grid in ${builtins.toString gridPackages}; do
if [ ! -d $grid ]; then
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this check redundant? The cp will fail if the directory doesn't exist.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this check doesn't exist, the error message is quite confusing

       > Running phase: installPhase
       > cp: missing destination file operand after '/nix/store/v3258iwkrrmh5iddli7sdzxgikiw3cyh-proj-data-1.16.0/'
       > Try 'cp --help' for more information.
```.

I prefer to leave it as is.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good. In that case I'd recommend changing the check to if ! [[ -d "$grid" ]] - it won't produce a syntax error if anyone includes an empty grid package by accident.

echo "ERROR: Grid ($grid) does not exist." >&2
exit 1
fi

cp $grid/!(*.sh|*.py) $out/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
cp $grid/!(*.sh|*.py) $out/
cp "$grid"/!(*.sh|*.py) "$out"/

That said, would it be possible to create a positive rather than negative glob to include the relevant files? Something like *.json *.tif *.tiff *.txt?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, but it is very hard to create future-proof list of allowed file types. I would prefer to leave it as is for now.

done

shopt -u extglob
runHook postInstall
'';

passthru.tests =
let
# build custom package containing `nz_linz` grids
projDataWithGrid = finalAttrs.finalPackage.overrideAttrs (_: {
postInstall = "cp nz_linz/* $out/";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this override gridPackages instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I would like to, but finalPackage doesn't support override (it supports only overrideAttrs). Any suggestions are welcome.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know Nix well enough to suggest anything. Feel free to ignore this, unless someone else has a suggestion.

});
in
{
proj-data = callPackage ./tests.nix {
proj-data = projDataWithGrid;
};
};

meta = with lib; {
description = "Repository for proj datum grids (for use by PROJ 7 or later)";
homepage = "https://proj4.org";
# Licensing note:
# All grids in the package are released under permissive licenses. New grids
# are accepted into the package as long as they are released under a license that
# is compatible with the Open Source Definition and the source of the grid is
# clearly stated and verifiable. Suitable licenses include:
# Public domain
# X/MIT
# BSD 2/3/4 clause
# CC0
# CC-BY (v3.0 or later)
# CC-BY-SA (v3.0 or later)
license = licenses.mit;
maintainers = teams.geospatial.members;
};
})
25 changes: 25 additions & 0 deletions pkgs/by-name/pr/proj-data/tests.nix
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
{ runCommand, proj, proj-data }:

let
inherit (proj-data) pname;
in
runCommand "${pname}-tests" { meta.timeout = 60; }
''
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test is using pipelines, so set -o pipefail might help.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, done.

set -o pipefail

# add proj-data files to proj resources search path
export PROJ_DATA=${proj}/share/proj:${proj-data}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this possibly be set by the package itself?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure what is the best approach. We can wrap all executables with PROJ_DATA path, but it wouldn't help with dozens of other apps using proj as library (like QGIS for example).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure what is the best approach. We can wrap all executables with PROJ_DATA path, [鈥

Sounds good.

but it wouldn't help with dozens of other apps using proj as library (like QGIS for example).

Isn't that unrelated? We'd be making one thing work without breaking anything else, right?


# conversion from NZGD1949 to NZGD2000 using proj strings
echo '173 -41 0' \
| ${proj}/bin/cs2cs --only-best -f %.8f \
+proj=longlat +ellps=intl +datum=nzgd49 +nadgrids=nz_linz_nzgd2kgrid0005.tif \
+to +proj=longlat +ellps=GRS80 +towgs84=0,0,0 \
| grep -E '[0-9\.\-]+*'

# conversion from NZGD1949 to NZGD2000 using EPSG codes
echo '-41 173 0' | ${proj}/bin/cs2cs --only-best -f %.8f EPSG:4272 EPSG:4167 \
| grep -E '[0-9\.\-]+*'

touch $out
''
12 changes: 10 additions & 2 deletions pkgs/development/libraries/proj/default.nix
Original file line number Diff line number Diff line change
@@ -1,18 +1,22 @@
{ lib
, stdenv
, callPackage
, fetchFromGitHub
, fetchpatch

# install grid resource files from proj-data package
, withProjData ? false

, cmake
, pkg-config
, buildPackages
, callPackage
, sqlite
, libtiff
, curl
, gtest
, nlohmann_json
, python3
, cacert
, proj-data
}:

stdenv.mkDerivation (finalAttrs: rec {
Expand Down Expand Up @@ -58,6 +62,10 @@ stdenv.mkDerivation (finalAttrs: rec {

doCheck = true;

postInstall = lib.optionalString withProjData ''
cp --recursive ${proj-data}/* $out/share/proj/
'';

passthru.tests = {
python = python3.pkgs.pyproj;
proj = callPackage ./tests.nix { proj = finalAttrs.finalPackage; };
Expand Down