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

[pdal] delete and replace different find modules #6603

Merged
merged 6 commits into from
Jun 3, 2019

Conversation

Neumann-A
Copy link
Contributor

makes linkage within pdal more robust.

@Neumann-A
Copy link
Contributor Author

ah this requires probably the geotiff pr #6596

@vicroms
Copy link
Member

vicroms commented May 24, 2019

#6596 has been merged to master
I'll trigger a new build.

@vicroms vicroms self-assigned this May 24, 2019
@vicroms
Copy link
Member

vicroms commented May 24, 2019

Results: pdal fails on x86-windows and x64-windows

Failure logs

@Neumann-A
Copy link
Contributor Author

Seems like it did not get the memo that geotiff has been updated. Will merge the master later to this pr. The logs clearly shows that it is the proj.lib vs proj target error.

@vicroms
Copy link
Member

vicroms commented May 25, 2019

/azp run

@vicroms
Copy link
Member

vicroms commented May 25, 2019

Looks like this time is pdal-c instead of pdal that's failing on x86- and x64- Windows.

I'm away from a PC, so I can't get the logs right now.

@cenit
Copy link
Contributor

cenit commented May 25, 2019

Again... not to complain but I am sure you know that like the geotiff patch this is already present in #6000 since some time...
Isn’t it better to join effort instead of resubmitting same patch in another pr? It just adds work for everybody. For you, for @vicroms and for me, that I have to merge master for nothing

@Neumann-A
Copy link
Contributor Author

not to complain but I am sure you know that like the geotiff patch this is already present in #6000 since some time...

I dont know that PR claims to fix LZMA and not geotiff and I am not actively watching all activtiy in vcpkg. I am just adding PRs to someday maybe get #5543 (or #6555) into vcpkg which will generally solve all link issues in regards to debug/release by injecting the correct vcpkg paths if necessary.

It just adds work for everybody.

#6608 (comment)

@Neumann-A
Copy link
Contributor Author

Regressions? The Preview CI does not show any.

@vicroms
Copy link
Member

vicroms commented May 30, 2019

All failures occurred in Mac OS, which is not yet on Preview CI.

@Rastaban can you provide logs for build 2715856?

@Neumann-A
Copy link
Contributor Author

since i am only touching pdal and the only port dependent on pdal is pdal-c there seems to be an error in the CI build

@microsoft microsoft deleted a comment from azure-pipelines bot May 30, 2019
@vicroms
Copy link
Member

vicroms commented May 30, 2019

/azp run

@vicroms
Copy link
Member

vicroms commented May 30, 2019

You're right, the CI results already looked suspicious once they only appeared on Mac OS.

@vicroms
Copy link
Member

vicroms commented Jun 3, 2019

Thanks for the PR!

@vicroms vicroms merged commit 35c8538 into microsoft:master Jun 3, 2019
@Neumann-A Neumann-A deleted the fix_pdal branch June 3, 2019 21:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants