-
-
Notifications
You must be signed in to change notification settings - Fork 14.8k
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
qgis: add server support #267301
qgis: add server support #267301
Conversation
2369f2a
to
1ad1fda
Compare
@ofborg build qgis qgis-ltr |
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/week-in-geospatial-team-11-17-dec-2023/37035/1 |
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/week-in-geospatial-team/37035/2 |
Result of 2 packages failed to build:
|
Getting a build error for
|
@nialov , thank you very much for reviewing
I think, the reason why
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/week-in-geospatial-team/37035/3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me! thanks for this, it's going to be really useful!
(lib.cmakeBool "WITH_PDAL" true) | ||
(lib.cmakeBool "WITH_QTWEBKIT" withWebKit) | ||
(lib.cmakeBool "WITH_SERVER" true) | ||
"-DQGIS_CGIBIN_SUBDIR=${placeholder "out"}/lib/cgi-bin" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For my culture, why do we need placeholder "out
here? $out
directly doesn't work?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if I remember correctly, but I guess just using $out
didn't work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not $(out)
?
Note: I've tested on the rebased version, build is passing and the executable is working fine (I tested only simple stuff: GetCapabilities and GetMap on a basic project). |
0df17ec
to
9dc5029
Compare
Great thanks for reviewing this PR. I rebased it on current master. The only remaining todo is to disable server by default and add some user friendly way to override it with |
Well I'd just propagate the argument:
Not entirely sure this works, as I'm still new to nixpkgs. I guess #259445 won't be of much help, since we need the wrap for python packages right? |
9dc5029
to
b0e306f
Compare
I'd prefer a solution that expose these parameters directly to the user:
Granted, it's a bit more boilerplate for us because we have to propagate every parameters, but I prefer that than to put the burden on users. Btw, I think we should also document these options in |
I agree.
Yes. |
Code looks good! Tested and works like expected. Thanks! |
Thanks for review. |
Description of changes
Add QGIS server support for QGIS and QGIS-LTR.
This PR is co-authored by @bgamari in #259303 .
Things done
nix.conf
? (See Nix manual)sandbox = relaxed
sandbox = true
nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD"
. Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/
)