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

mapnik: unstable-2022-10-18 -> unstable-2023-11-28 #277128

Merged
merged 5 commits into from
Jan 3, 2024

Conversation

hummeltech
Copy link
Contributor

@hummeltech hummeltech commented Dec 27, 2023

Description of changes

  • Removed no-longer-needed patch:

    • include.patch: Fixed here
  • Added patch to account for FONTS_INSTALL_DIR/PLUGINS_INSTALL_DIR containing full paths, causing this issue:

    Broken paths found in a .pc file! /nix/store/...-mapnik-unstable-2023-11-28/lib/pkgconfig/libmapnik.pc
    The following lines have issues (specifically '//' in paths).
    5:fonts_dir=${prefix}//nix/store/...-mapnik-unstable-2023-11-28/lib/mapnik/fonts
    6:plugins_dir=${prefix}//nix/store/...-mapnik-unstable-2023-11-28/lib/mapnik/input
    
  • Enabled Checks, which requires a patch to allow one test to fail:

    • datasource-ogr-test-should-fail.patch
  • Updated python-mapnik to latest upstream

    • Based on the proj6 branch (which currently has an open pull request)

This "release" also includes a patch which mod_tile is expecting for Mapnik versions >= 4:
Seen here

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.05 Release Notes (or backporting 23.05 and 23.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

Copy link
Member

@dotlambda dotlambda left a comment

Choose a reason for hiding this comment

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

Please squash.

@hummeltech hummeltech force-pushed the MapnikUpgrade branch 2 times, most recently from a8ad87a to a381959 Compare December 27, 2023 15:59
@hummeltech hummeltech requested a review from dotlambda December 27, 2023 16:17
Copy link
Member

@dotlambda dotlambda left a comment

Choose a reason for hiding this comment

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

The first line of commit message should be mapnik: unstable-2022-10-18 -> unstable-2023-11-28. You can mention the removed patch in the remainder.

@dotlambda
Copy link
Member

Result of nixpkgs-review pr 277128 run on x86_64-linux 1

10 packages failed to build:
  • apacheHttpdPackages.mod_tile (apacheHttpdPackages_2_4.mod_tile)
  • mapnik
  • perl536Packages.Tirex
  • perl536Packages.Tirex.devdoc
  • perl538Packages.Tirex
  • perl538Packages.Tirex.devdoc
  • python310Packages.python-mapnik
  • python310Packages.python-mapnik.dist
  • python311Packages.python-mapnik
  • python311Packages.python-mapnik.dist

@hummeltech
Copy link
Contributor Author

hummeltech commented Dec 28, 2023

Hmm, yes I see that. I am not having any issues building it locally, and the logs are not showing any errors (which could indicate a visit by the OOM Killer). I have added a commit in order to attempt the build with enableParallelBuilding disabled, which is what previous versions of Mapnik did in order to build certain targets which required more RAM. And since the x86_64-darwin builds (which use -j1) are all building successfully, I think this might be a valid path. I cannot think of anything else to try at this time.

@hummeltech
Copy link
Contributor Author

hummeltech commented Dec 28, 2023

Nevermind, upon closer inspection of the logs, it looks like there is still an issue which requires cmake-harfbuzz.patch. I have re-included that patch. Not sure why it doesn't affect me.

@dotlambda
Copy link
Member

Result of nixpkgs-review pr 277128 run on x86_64-linux 1

4 packages failed to build:
  • python310Packages.python-mapnik
  • python310Packages.python-mapnik.dist
  • python311Packages.python-mapnik
  • python311Packages.python-mapnik.dist
6 packages built:
  • apacheHttpdPackages.mod_tile (apacheHttpdPackages_2_4.mod_tile)
  • mapnik
  • perl536Packages.Tirex
  • perl536Packages.Tirex.devdoc
  • perl538Packages.Tirex
  • perl538Packages.Tirex.devdoc

@hummeltech
Copy link
Contributor Author

hummeltech commented Dec 29, 2023

I'm not sure why that was no longer working, I added python3Packages.boost to python-mapnik's propagatedBuildInputs as that being missing seemed to be causing the library (libboost_python310.so) to be missing, I.E.:

/nix/store/...-binutils-2.40/bin/ld: cannot find -lboost_python310: No such file or directory

hummeltech and others added 2 commits December 29, 2023 00:44
Removed no-longer-needed patch:
* `include.patch`: Fixed [here](mapnik/mapnik@64a031a)

Added patch to account for `FONTS_INSTALL_DIR`/`PLUGINS_INSTALL_DIR`
containing full paths, causing this issue:
```
Broken paths found in a .pc file! /nix/store/lriysmfydb9p6g6s02q6ri24nzwmi494-mapnik-unstable-2023-11-28/lib/pkgconfig/libmapnik.pc
The following lines have issues (specifically '//' in paths).
5:fonts_dir=${prefix}//nix/store/lriysmfydb9p6g6s02q6ri24nzwmi494-mapnik-unstable-2023-11-28/lib/mapnik/fonts
6:plugins_dir=${prefix}//nix/store/lriysmfydb9p6g6s02q6ri24nzwmi494-mapnik-unstable-2023-11-28/lib/mapnik/input
```

Enabled `Checks`, which requires a patch to allow one test to fail:
* `datasource-ogr-test-should-fail.patch`
@dotlambda
Copy link
Member

I don't think it has anything to do with adding boost to propagatedBuildInputs. I get errors like

src/mapnik_enumeration.hpp:71:20: error: invalid ‘static_cast’ from type ‘const mapnik::enumeration<mapnik::gamma_method_enum, mapnik::gamma_method_e_to_string, mapnik::gamma_method_e_from_string, mapnik::gamma_method_e_lookup>’ to type ‘long int’
   71 |                 ,  static_cast<long>( v ));
      |                    ^~~~~~~~~~~~~~~~~~~~~~

and

src/mapnik_enumeration.hpp:71:20: error: invalid ‘static_cast’ from type ‘const mapnik::enumeration<mapnik::gamma_method_enum, mapnik::gamma_method_e_to_string, mapnik::gamma_method_e_from_string, mapnik::gamma_method_e_lookup>’ to type ‘long int’
   71 |                 ,  static_cast<long>( v ));
      |                    ^~~~~~~~~~~~~~~~~~~~~~

I think python-mapnik is simply not yet compatible with recent versions of mapnik.
I suggest you open an upstream issue and mark python-mapnik broken.

@dotlambda
Copy link
Member

mapnik/python-mapnik#270 (comment) claims to have a fix but after applying that commit using fetchpatch it complains about C++20.

@hummeltech
Copy link
Contributor Author

hummeltech commented Jan 1, 2024

Alright @dotlambda, I figured out the python-mapnik issue and I now have nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD" running without errors on my machine. Let me know if it's working for you.

It looks like support for Proj >= 6 isn't completely merged into the default branch, so I tried out the proj6 branch (omitting the latest commits which break the build,) and everything builds again successfully.
mapnik/python-mapnik@master...proj6

The patch from your previous comment is included in the aforementioned branch.

Copy link
Member

@dotlambda dotlambda left a comment

Choose a reason for hiding this comment

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

The last commit message should be python311Packages.python-mapnik: unstable-2020-09-08 -> unstable-2023-02-23.

@dotlambda
Copy link
Member

@hummeltech Do you want to add yourself as maintainer?

@hummeltech
Copy link
Contributor Author

Sure, that's fine, shall I add myself onto the list?

@dotlambda
Copy link
Member

Sure, that's fine, shall I add myself onto the list?

Yes, you'll first have to add yourself to maintainers/maintainer-list.nix in one commit and then to mapnik's meta.maintainers in a separate one.

@ofborg ofborg bot added the 11.by: package-maintainer This PR was created by the maintainer of the package it changes label Jan 2, 2024
@dotlambda
Copy link
Member

Result of nixpkgs-review pr 277128 run on x86_64-linux 1

10 packages built:
  • apacheHttpdPackages.mod_tile (apacheHttpdPackages_2_4.mod_tile)
  • mapnik
  • perl536Packages.Tirex
  • perl536Packages.Tirex.devdoc
  • perl538Packages.Tirex
  • perl538Packages.Tirex.devdoc
  • python310Packages.python-mapnik
  • python310Packages.python-mapnik.dist
  • python311Packages.python-mapnik
  • python311Packages.python-mapnik.dist

Copy link
Member

@dotlambda dotlambda left a comment

Choose a reason for hiding this comment

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

Darwin fails but I don't have access to the darwin builder atm so I won't be able to fix it. That shouldn't stop us from updating though.

@dotlambda dotlambda merged commit 07e3d3d into NixOS:master Jan 3, 2024
23 checks passed
@hummeltech hummeltech deleted the MapnikUpgrade branch January 3, 2024 14:57
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.

2 participants