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

[vcpkg] fatal_error when patch fails to apply #8087

Merged
merged 181 commits into from
Oct 7, 2019

Conversation

cenit
Copy link
Contributor

@cenit cenit commented Sep 7, 2019

Continued work from #6578

@cenit
Copy link
Contributor Author

cenit commented Sep 7, 2019

Quoting from previous pr:

This pr MUST NOT CONTAIN any improvement nor any update to any port. It is just a review of portfiles to make them adhere better to CMake standards and vcpkg's best practices, towards the goal of enabling a FATAL_ERROR for unmerged patches.
Any help is very appreciated, just let me know if you want to participate

edit: of course, any improvement that is necessary for the "green badge" is valid for this PR

@cenit
Copy link
Contributor Author

cenit commented Sep 7, 2019

Wondering why a change in a common script does not trigger a whole rebuild? @Rastaban

I will continue the work without the support of CI, which I was hoping to exploit

@cenit
Copy link
Contributor Author

cenit commented Oct 7, 2019

bumping all the regressions, one by one in order to better understand the problems, was enough to solve most of them. A couple required some more attention and were fixed along the way.

@vicroms, I think that this PR is now in the best possible situation for a merge: no regression, many many ports fixed along the way (new passes, yay), and patches/upgrades cannot be accepted anymore if they break a patch or do not apply them correctly (which was the specific target of this PR).
Since it is extremely broad in spectrum, keeping it aligned with master is very time consuming and I'd hope to not have to do for a long time... I hope you can understand.

@cbezault
Copy link
Contributor

cbezault commented Oct 7, 2019

I've reviewed all this files. It's a lot and so I might have missed something but it looks good to me.

@cbezault cbezault merged commit 726c111 into microsoft:master Oct 7, 2019
@cenit
Copy link
Contributor Author

cenit commented Oct 7, 2019

Thanks a lot @cbezault
I tagged @vicroms because I misunderstood you already changed project
Glad you were still on it :)

@cenit cenit deleted the dev/cenit/modernize branch October 7, 2019 19:49
@cbezault
Copy link
Contributor

cbezault commented Oct 8, 2019

I'm still around just winding down.

@vicroms
Copy link
Member

vicroms commented Oct 8, 2019

You cannot escape

@cenit
Copy link
Contributor Author

cenit commented Oct 8, 2019

I will try my best for that to happen, locking you here inside monstrous PRs

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.

5 participants