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

[many ports] improvements for linux/wsl #6730

Merged
merged 86 commits into from
Jun 21, 2019
Merged

Conversation

cenit
Copy link
Contributor

@cenit cenit commented Jun 2, 2019

No description provided.

[clapack] better integration with linux environment
[visit-struct] put cmake config file in the expected folder
[geogram] remove trailing underscore to enable compatibility with OpenBLAS
@cenit
Copy link
Contributor Author

cenit commented Jun 4, 2019

finally fixed the clapack very bad regression on linux introduced in #6542

Copy link
Contributor

@cbezault cbezault left a comment

Choose a reason for hiding this comment

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

Could we have the libusb changes as its own PR.
Could we also get all the changes associated with BLAS/LAPACK in its own PR.

@cenit
Copy link
Contributor Author

cenit commented Jun 5, 2019

I am still fixing things up, I will start thinking about splitting commits later, if ever.
To be honest, now it's a moment of crysis with vcpkg because I am loosing months of work fixing things up (I think recent PRs of mine are here to demonstrate it) and it seems there's no end, also because broken PRs are still creeping in breaking stuff (lapack/openblas are totally broken now on master.............).
Finally CI has been introduced but it is still VERY insufficient (this tragedy of build_tests always disabled means that we don't even check library downstream usability even with self-included tests.... we only check if it builds but you can understand it's a VERY broken assumption).

Just let me finish this work, then do whatever you want to merge what you need.
I was fixing unnecessary/broken paths here and there while waiting for builds to finish during the night, IMHO just a thorough human read and analysis should be enough to verify it and ease a lot of work to disentangle commits that were meant to make things work...

@cenit
Copy link
Contributor Author

cenit commented Jun 19, 2019

I'd like to consider this work as done, in order to be ready for OpenCV 4...

@cbezault
Copy link
Contributor

And I don't have the heart to pick the changes out into different prs. I'll audit the changes and merge once we get a green check.

@cenit
Copy link
Contributor Author

cenit commented Jun 19, 2019

@cbezault ok, seems reasonable.
CI on osx is requiring an unexpected extremely long time, don't know why. Apart from that, Linux is ok and windows has a regression for ffmpeg on x64-uwp which looks like a CI problem: MSYS just melts down at the beginning, being unable to install some packages required to work... also, I can't reproduce it locally. I already tried many times to trigger a new build, the scary thing is that it always fails at the MSYS initial setup?!??

@cenit
Copy link
Contributor Author

cenit commented Jun 20, 2019

@Rastaban maybe you know better what's going on here on windows.
Also, I found out that the logic behind CI is not so clear. I was sure that if B depended on A, modifying A would trigger a rebuild of B, but this is not always the case, I had to manually trigger a rebuild of B to have its regression fixed thanks to the fix in A

@cbezault
Copy link
Contributor

The Pacman error is a known issue that we're trying to fix permanently. It has to do with a lock file not getting deleted properly.

Modifying A should retrigger a build of B unless you changed A back to a version that has already existed in the past. Anything else is a bug.

@cenit
Copy link
Contributor Author

cenit commented Jun 20, 2019

you can just see the last two commits... just triggering rebuild of ports seems enough to fix them... I modified their dependencies to fix them (the bug was not in the ports themselves but in their dependencies), but it seems that they were not rebuilt and I had to do it manually.
Without documentation i also realized that reasonable rule, but somehow it does not work always

@cbezault
Copy link
Contributor

I'll look into it once I'm in today. It's concerning.

@cbezault
Copy link
Contributor

Also don't bother retriggeing uwp.

@cenit
Copy link
Contributor Author

cenit commented Jun 20, 2019

green mark? :)

Please let me know any other review request you might have. Also because #5169 is ready, so I'd like to close this work

Copy link
Contributor

@cbezault cbezault left a comment

Choose a reason for hiding this comment

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

Sorry to slow this down.

ports/azure-storage-cpp/CONTROL Outdated Show resolved Hide resolved
ports/botan/portfile.cmake Outdated Show resolved Hide resolved
ports/ceres/CONTROL Outdated Show resolved Hide resolved
ports/ceres/CONTROL Outdated Show resolved Hide resolved
ports/cpr/CONTROL Outdated Show resolved Hide resolved
ports/magnum-extras/CONTROL Outdated Show resolved Hide resolved
ports/magnum-integration/CONTROL Outdated Show resolved Hide resolved
ports/magnum-plugins/CONTROL Outdated Show resolved Hide resolved
ports/openmvs/CONTROL Outdated Show resolved Hide resolved
@cenit
Copy link
Contributor Author

cenit commented Jun 20, 2019

No problem. Better to fix problems now before it’s too late. Just wanted to avoid wasted time, but review requests are never wasted time

@cbezault cbezault merged commit 47d206e into microsoft:master Jun 21, 2019
@cenit cenit deleted the dev/cenit/coin branch June 21, 2019 04:38
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.

6 participants