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

[asmjit] Update to the latest commit #12524

Merged
merged 6 commits into from
Aug 7, 2020

Conversation

ZehMatt
Copy link
Contributor

@ZehMatt ZehMatt commented Jul 21, 2020

Updates the asmjit port to the latest commit in the master branch which contains some bug fixes.

@ghost
Copy link

ghost commented Jul 21, 2020

CLA assistant check
All CLA requirements met.

@NancyLi1013 NancyLi1013 added the category:port-update The issue is with a library, which is requesting update new revision label Jul 22, 2020
ports/asmjit/CONTROL Outdated Show resolved Hide resolved
@NancyLi1013
Copy link
Contributor

NancyLi1013 commented Jul 22, 2020

@ZehMatt
Sorry, I ignored this point before.
Could you please also remove these code form ci.baseline.txt?

asmjit:arm64-windows=fail
asmjit:arm-uwp=fail

Since we have added Supports:!uwp in CONTROL file, there is no need to add these failed info.

@ZehMatt
Copy link
Contributor Author

ZehMatt commented Jul 22, 2020

Done

@NancyLi1013
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@NancyLi1013
Copy link
Contributor

Hi @ZehMatt
Could you please look into the regressions for ompl and polyhook2?

@NancyLi1013
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@ZehMatt
Copy link
Contributor Author

ZehMatt commented Jul 30, 2020

Polyhook2 uses a now deprecated function. I can not update Polyhook2 before updating AsmJit and I can not update AsmJit before updating Polyhook2, how would I go about this problem?

@NancyLi1013
Copy link
Contributor

Can you update them at the same time in this PR?

@ZehMatt
Copy link
Contributor Author

ZehMatt commented Aug 4, 2020

The author of Polyhook2 updated to the most recent asmjit commit, I've included it now in this PR.

ports/polyhook2/CONTROL Show resolved Hide resolved
@NancyLi1013
Copy link
Contributor

Hi @ZehMatt

Thanks for this PR.
Have you tested these features in this PR?

@ZehMatt
Copy link
Contributor Author

ZehMatt commented Aug 5, 2020

Hi @ZehMatt

Thanks for this PR.
Have you tested these features in this PR?

The asmjit port I use heavily, Polyhook2 not so much. So yes 🤔?

@stevemk14ebr
Copy link
Contributor

stevemk14ebr commented Aug 5, 2020

Hi i'm the author of polyhook, the latest asmjit changes have been tested by me and work fine with the latest polyhook now that i've upgraded. @ZehMatt i'm not sure what the purpose of ci.baseline.txt is for, but i think we actually want polyhook in there? To update the polyhook port please simply replace the REF and SHA256 in the portfile with:

REF  69fa86df9ae125617ac660b2d6ae2920c69194d9
SHA512 822c6f07106b5264ab0fe6608875e18ff85572e4316f9bf90be9c68a2c0ed2c4a8f1d1b9fd497d8adf8420c1c9cc34ff46f2e8848f128e37491a86212ed14dc9

Then additionally in the CONTROL set the version to Version: 2020-08-03. Do not change any other values in the polyhook2 port, everything else will work fine once you upgrade the version and commit REF.

Since this is your PR could you do that?

@ZehMatt
Copy link
Contributor Author

ZehMatt commented Aug 5, 2020

I've already updated Polyhook2, and I was specifically asked to remove the lines out of ci.baseline.txt, it seems to be deprecated.

@NancyLi1013
Copy link
Contributor

@stevemk14ebr

Thanks for your so detailed information about polyhook and also for your support and help.
Since you have tested these two ports and their features, they work fine, there is no need to test it again now.

For ci.baseline.txt, it is used to keep CI work fine. Since we should make sure all checks pass on CI(show green mark) before every PR is merged. So if some ports don't support any specific triplets and there is no description for the unsupported triplet in Supports filed, we need to set the triplet as fail in ci.baseline.txt.
You can see this file here. https://github.com/microsoft/vcpkg/blob/master/scripts/ci.baseline.txt

@ZehMatt

Thanks for this PR. It seems that all checks have passed on CI.
LGTM now.

@NancyLi1013 NancyLi1013 added info:reviewed Pull Request changes follow basic guidelines and removed requires:author-response labels Aug 6, 2020
@strega-nil strega-nil merged commit 15141fb into microsoft:master Aug 7, 2020
@ZehMatt ZehMatt deleted the update-asmjit branch August 8, 2020 19:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:port-update The issue is with a library, which is requesting update new revision info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants