Fix PDAL \ untwine on windows #178

SrNetoChan · 2021-03-06T16:16:38Z

Checklist

Used a personal fork of the feedstock to propose changes
Bumped the build number (if the version is unchanged)
Reset the build number to 0 (if the version changed)
Re-rendered with the latest conda-smithy (Use the phrase @conda-forge-admin, please rerender in a comment in this PR for automated rerendering)
Ensured the license file is being packaged.

Trying to finish #174 work, which is failing on windows because of untwine.

conda-forge-linter · 2021-03-06T16:16:43Z

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

SrNetoChan · 2021-03-06T16:34:28Z

@gillins this is the base for fixing untwine build. From what I was able to understand untwine replaces entwine. Maybe @hobu can point us in the right direction.

hobu · 2021-03-06T16:36:38Z

Untwine does not replace Entwine. It is an alternative generator that has some features that are more useful in the context of desktop software like QGIS:

Constrained memory usage exchanged for building a local disk cache
Bottom-up instead of top-down building of the tree
Less features than Entwine (not all encodings supported, no addons, etc)
Parallelization is left to the user

SrNetoChan · 2021-03-06T16:59:31Z

So, it replaces entwine under QGIS context, right? Or there is any benefit in also installing entwine alongside QGIS?

hobu · 2021-03-06T17:14:05Z

So, it replaces entwine under QGIS context, right?

It should produce 'equivalent' output under a different usage scenario – the need to constrain memory usage. The cost of this behavior is a local cache of the data as it is built.

Or there is any benefit in also installing entwine alongside QGIS?

If you wanted some of the other features that entwine provides it would be useful. QGIS can open Entwine-created EPT collections the same as Untwine.

conda-forge-linter · 2021-03-06T22:28:27Z

Hi! This is the friendly automated conda-forge-linting service.

I wanted to let you know that I linted all conda-recipes in your PR (recipe) and found some lint.

Here's what I've got...

For recipe:

Selectors are suggested to take a <two spaces>#<one space>[<expression>] form. See lines [11]

conda-forge-linter · 2021-03-06T22:31:38Z

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

This reverts commit 021d51a.

This reverts commit 488f498.

gillins · 2021-03-11T23:12:55Z

Finally managed to get some time to boot my old Windows PC and have a look at this. Turns out that #including Windows.h brings in winsock.h no matter what you do unless you #define WIN32_LEAN_AND_MEAN. I have a workaround in (https://github.com/conda-forge/qgis-feedstock/pull/178/files#diff-913bcc89317907aa358d649f080993946cc0b6b6373b7f44df9f35bf4653b212)

@hobu we need your advice on the below:

You already define WIN32_LEAN_AND_MEAN in the standalone untwine (https://github.com/hobu/untwine/blob/main/cmake/win32_compiler_options.cmake#L9) - should we try and upstream back to qgis? Not sure if some of the other options you use should go back there also?
I had to create another patch (https://github.com/conda-forge/qgis-feedstock/pull/178/files#diff-a93dca7189807151286b0a389f6700262c034808b63d06144344c4085eae88c7) to fix MapFile.cpp on Windows. Does this make sense? I can submit this back to untwine (and also qgis) if you like?
I'm now getting weird C++ errors I don't understand (only with MSVC) in FileProcessor.cpp (https://dev.azure.com/conda-forge/feedstock-builds/_build/results?buildId=288844&view=logs&j=00f5923e-fdef-5026-5091-0d5a0b3d5a2c&t=3cc4a9ed-60e1-5810-6eb3-5f9cd4a26dba&l=3027). Is there a simple fix, or should we disable untwine for Windows?

Should we be building untwine in conda-forge and just linking to it (is it likely other applications will use it?), or is building this way fine for now?

hobu · 2021-03-11T23:42:44Z

@abellgithub will have to chime in here on the specifics of Untwine building. We haven't seen any of these issues, but again we're not the ones embedding it into a bigger application either.

ocefpaf · 2021-03-12T16:50:43Z

Finally managed to get some time to boot my old Windows PC and have a look at this.

@gillins conda-forge got a donation from OVH for Windows "cloud" Windows machine. It is quite robust and it is what we are using to build Qt. If that would help you with qgis please let me know and we can work something out to give you access to it.

gillins · 2021-03-13T04:42:43Z

If that would help you with qgis please let me know

Thanks @ocefpaf will let you know. I need a Windows machine going anyway for work stuff so probably ok ATM. If we need to build qgis ourselves regularly (i.e. if it started timing out on the CIs) this could be a good solution as my internet is rubbish...

SrNetoChan · 2021-03-22T07:36:08Z

@gillins should I rebase all commits that have been merged meanwhile?

gillins · 2021-03-22T07:42:19Z

Let's wait until they get back to us. I suspect they'll say Windiws isn't supported yet so we we will likely have to wait for a future version anyway...

SrNetoChan · 2021-03-22T08:00:40Z

Got lost, who are "they"?

gillins · 2021-03-22T10:32:02Z

Got lost, who are "they"?

Sorry, I didn't make that clear. I meant the untwine maintainers that were pinged above (see #178 (comment))

hobu · 2021-03-22T13:40:57Z

Are you waiting for patches to fix these windows issues? We don't have the cycles to do the work on that topic at the moment, but we would take patches and cut a release if you or someone made PRs to fix them.

hobu · 2021-03-22T15:02:51Z

Jürgen was able to make an OSGeo4W release with Untwine support, so I think the issue here must be the discrepancy between compiler settings of whatever he is using and Conda's.

SrNetoChan · 2021-03-22T15:20:01Z

@hobu I think @gillins was referring to some hint of what could be wrong, not a fix.

jef-n · 2021-03-23T09:28:50Z

Thanks for the patches @jef-n - I'll use these as a guide and see if I can get it all compiling. Are you planning to upstream these?

Sure, once the old osgeo4w is replaced by the new one - the previous vc didn't need these changes (and I didn't verify whether it builds with them).

gillins · 2021-03-24T01:23:08Z

@jef-n I'm still getting weird C++ errors I don't understand, plus I need to make different changes from what you have to get it that far. There must be differences in the versions/environments that we are using. I see @abellgithub has already incorporated one of the changes I had to make (that you didn't): hobuinc/untwine@c7f94c1

So I am all a bit confused. I'm going to close this and wait for a future version of untwine to be incorporated in qgis before trying again. If anyone feels they can get this working, please go ahead!

abellgithub · 2021-03-24T01:35:13Z

If you post a link to the error output, I can take a look.

gillins · 2021-03-24T01:37:33Z

https://dev.azure.com/conda-forge/feedstock-builds/_build/results?buildId=294057&view=logs&j=5be07ae1-d8ba-5406-47b6-8e3a3a12f825&t=0bf03e01-0bec-5b85-5316-b1633322e895&l=1805

..\external\untwine\epf\FileProcessor.cpp(73): error C3493: 'count' cannot be implicitly captured because no default capture mode has been specified

abellgithub · 2021-03-24T02:13:04Z

This is a bug in VS 2017. Things work in VS 2019. I'm not sure how to work around it but I'll give it some thought.

…

On Tue, Mar 23, 2021 at 9:37 PM Sam Gillingham ***@***.***> wrote: https://dev.azure.com/conda-forge/feedstock-builds/_build/results?buildId=294057&view=logs&j=5be07ae1-d8ba-5406-47b6-8e3a3a12f825&t=0bf03e01-0bec-5b85-5316-b1633322e895&l=1805 ..\external\untwine\epf\FileProcessor.cpp(73): error C3493: 'count' cannot be implicitly captured because no default capture mode has been specified — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#178 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAKBMMFC52M4A3ASYFXT2ETTFE66XANCNFSM4YW3YBXA> .

-- Andrew Bell ***@***.***

SrNetoChan · 2021-03-24T16:52:17Z

@gillins said:

So I am all a bit confused. I'm going to close this and wait for a future version of untwine to be incorporated in qgis before trying again. If anyone feels they can get this working, please go ahead!

Fair enough! Thanks for all your efforts Sam.

gillins · 2021-04-13T03:22:38Z

@abellgithub Did you have any thoughts on this? We are tied to particular versions of VS to keep compatibility with Python....

hobu · 2021-04-13T15:09:30Z

hobuinc/untwine#57 fixed this issue. Grab the latest main branch from https://github.com/hobu/untwine/ and it should work.

gillins · 2021-04-13T23:13:34Z

@conda-forge-admin, please rerender

…nda-forge-pinning 2021.04.13.13.54.57

SrNetoChan · 2021-04-13T23:49:10Z

SrNetoChan · 2021-04-16T22:38:28Z

@gillins there's a new 3.18.2 version, which was already merged into master, but with pdal disabled for windows. Should I try to bump it here, or create a new PR for it, and if needed we try to cherry pick the patches?

conda-forge-linter · 2021-04-17T07:09:04Z

Hi! This is the friendly automated conda-forge-linting service.

I was trying to look for recipes to lint for you, but it appears we have a merge conflict.
Please try to merge or rebase with the base branch to resolve this conflict.

Please ping the 'conda-forge/core' team (using the @ notation in a comment) if you believe this is a bug.

conda-forge-linter · 2021-04-17T13:48:02Z

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

gillins · 2021-04-17T23:14:47Z

Thanks @SrNetoChan , hopefully I'll have some time to look into this during the week

SrNetoChan · 2021-04-19T13:23:50Z

What, wait, did it work?!

What a flight!! What fixed it?

Thanks for all your work!! @gillins

gillins · 2021-04-19T21:57:40Z

Trick was to define WIN32_LEAN_AND_MEAN plus including shellapi.h in qgisapp.h to workaround this not being included as part of windows.h... I'll try and get this upstream to QGIS as soon as I can.

Fix PDAL \ untwine on windows

2006683

SrNetoChan requested review from ceholden, gillins, kmuehlbauer and ocefpaf as code owners March 6, 2021 16:16

Add patch to replace winsock by winsock2

488f498

Fix linter message

021d51a

SrNetoChan and others added 4 commits March 6, 2021 23:01

Revert "Fix linter message"

83b466b

This reverts commit 021d51a.

Revert "Add patch to replace winsock by winsock2"

570b01e

This reverts commit 488f498.

attempt to fix various windows errors

77840a9

add fcntl.h

4875b1c

gillins mentioned this pull request Mar 20, 2021

qgis v3.18.1 #180

Merged

3 tasks

gillins closed this Mar 24, 2021

gillins reopened this Apr 13, 2021

update patch

97ad0a1

MNT: Re-rendered with conda-build 3.21.4, conda-smithy 3.10.0, and co…

7f14ffb

…nda-forge-pinning 2021.04.13.13.54.57

gillins added 3 commits April 14, 2021 11:14

add patch for missing include

532a092

add another WIN32_LEAN_AND_MEAN

dbdffce

try a different approach

f479708

Bump version to 3.18.2

1922d86

Merge branch 'master' into fix_pdal_windows

6e416b9

tidy patches and go back to original approach

af619a7

gillins mentioned this pull request Apr 19, 2021

add #include <algorithm> to QgisUntwine_win.cpp hobuinc/untwine#59

Merged

gillins merged commit ec221f9 into conda-forge:master Apr 19, 2021

gillins mentioned this pull request Apr 22, 2021

fixes for compiling with PDAL/untwine with MSVC/Windows qgis/QGIS#42904

Closed

SrNetoChan deleted the fix_pdal_windows branch May 30, 2021 23:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix PDAL \ untwine on windows #178

Fix PDAL \ untwine on windows #178

SrNetoChan commented Mar 6, 2021

conda-forge-linter commented Mar 6, 2021

SrNetoChan commented Mar 6, 2021

hobu commented Mar 6, 2021

SrNetoChan commented Mar 6, 2021

hobu commented Mar 6, 2021

conda-forge-linter commented Mar 6, 2021

conda-forge-linter commented Mar 6, 2021

gillins commented Mar 11, 2021

hobu commented Mar 11, 2021

ocefpaf commented Mar 12, 2021

gillins commented Mar 13, 2021

SrNetoChan commented Mar 22, 2021

gillins commented Mar 22, 2021

SrNetoChan commented Mar 22, 2021

gillins commented Mar 22, 2021

hobu commented Mar 22, 2021

hobu commented Mar 22, 2021

SrNetoChan commented Mar 22, 2021

jef-n commented Mar 23, 2021

gillins commented Mar 24, 2021

abellgithub commented Mar 24, 2021 •

edited

Loading

gillins commented Mar 24, 2021

abellgithub commented Mar 24, 2021 via email

SrNetoChan commented Mar 24, 2021

gillins commented Apr 13, 2021

hobu commented Apr 13, 2021

gillins commented Apr 13, 2021

SrNetoChan commented Apr 13, 2021

SrNetoChan commented Apr 16, 2021

conda-forge-linter commented Apr 17, 2021

conda-forge-linter commented Apr 17, 2021

gillins commented Apr 17, 2021

SrNetoChan commented Apr 19, 2021

gillins commented Apr 19, 2021

Fix PDAL \ untwine on windows #178

Fix PDAL \ untwine on windows #178

Conversation

SrNetoChan commented Mar 6, 2021

conda-forge-linter commented Mar 6, 2021

SrNetoChan commented Mar 6, 2021

hobu commented Mar 6, 2021

SrNetoChan commented Mar 6, 2021

hobu commented Mar 6, 2021

conda-forge-linter commented Mar 6, 2021

conda-forge-linter commented Mar 6, 2021

gillins commented Mar 11, 2021

hobu commented Mar 11, 2021

ocefpaf commented Mar 12, 2021

gillins commented Mar 13, 2021

SrNetoChan commented Mar 22, 2021

gillins commented Mar 22, 2021

SrNetoChan commented Mar 22, 2021

gillins commented Mar 22, 2021

hobu commented Mar 22, 2021

hobu commented Mar 22, 2021

SrNetoChan commented Mar 22, 2021

jef-n commented Mar 23, 2021

gillins commented Mar 24, 2021

abellgithub commented Mar 24, 2021 • edited Loading

gillins commented Mar 24, 2021

abellgithub commented Mar 24, 2021 via email

SrNetoChan commented Mar 24, 2021

gillins commented Apr 13, 2021

hobu commented Apr 13, 2021

gillins commented Apr 13, 2021

SrNetoChan commented Apr 13, 2021

SrNetoChan commented Apr 16, 2021

conda-forge-linter commented Apr 17, 2021

conda-forge-linter commented Apr 17, 2021

gillins commented Apr 17, 2021

SrNetoChan commented Apr 19, 2021

gillins commented Apr 19, 2021

abellgithub commented Mar 24, 2021 •

edited

Loading