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 baseline] [vs-yasm] Build yasm instead of downloading it to work around memory corruption bugs in yasm itself. #14003

Merged
merged 20 commits into from
Oct 28, 2020

Conversation

BillyONeal
Copy link
Member

Works around yasm/yasm#153 .

Should resolve #12237 .

@PhoebeHui PhoebeHui self-assigned this Oct 13, 2020
@PhoebeHui PhoebeHui added the category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist label Oct 13, 2020
C:\Dev\vcpkg\buildtrees\gmp\x64-windows-static-rel\adcc6b3cc4-2b6258d695.clean\SMP\obj\Release\x64\libgmp\addaddmul_1msb0.obj : fatal error LNK1106: invalid file or disk full: cannot seek to 0x23B

:(
@BillyONeal
Copy link
Member Author

/azp run

@azure-pipelines
Copy link

No pipelines are associated with this pull request.

@BillyONeal
Copy link
Member Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@BillyONeal
Copy link
Member Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@BillyONeal
Copy link
Member Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

…_find_acquire_program(YASM) with yasm-tool-helper
@strega-nil
Copy link
Contributor

This seems like an issue for cross-compilation; have you considered the fallout there?

@BillyONeal
Copy link
Member Author

@strega-nil Yes, but I don't see another way. We can't be shipping ports which fail every 2 seconds because their build tools have memory corruption bugs.

@strega-nil
Copy link
Contributor

hm. I don't like it but yeah, you're right.

We seriously need to ship tool ports.

@PhoebeHui PhoebeHui changed the title [vs-yasm] Build yasm instead of downloading it to work around memory corruption bugs in yasm itself. [vcpkg baseline] [vs-yasm] Build yasm instead of downloading it to work around memory corruption bugs in yasm itself. Oct 28, 2020
Copy link
Contributor

@ras0219 ras0219 left a comment

Choose a reason for hiding this comment

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

LGTM once comments are addressed

@@ -1277,6 +1267,7 @@ namespace vcpkg::Files
#ifdef _WIN32
fs::path win32_fix_path_case(const fs::path& source)
{
using fs::is_slash;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
using fs::is_slash;

since the use below is qualified?

Copy link
Member Author

Choose a reason for hiding this comment

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

The use below is not qualified:

if (last - first >= 3 && is_slash(first[0]) && is_slash(first[1]) && !is_slash(first[2]))

GitHub's review tool failed you.

@@ -0,0 +1,37 @@
vcpkg_fail_port_install(MESSAGE "The yasm-tool port currently only supports Windows" ON_TARGET "Linux" "OSX" ON_ARCH "x64" "arm")
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
vcpkg_fail_port_install(MESSAGE "The yasm-tool port currently only supports Windows" ON_TARGET "Linux" "OSX" ON_ARCH "x64" "arm")
vcpkg_fail_port_install(MESSAGE "The yasm-tool port is only intended to be built for x86 Windows" ON_TARGET "Linux" "OSX" ON_ARCH "x64" "arm")

@@ -9,14 +9,15 @@ if(VCPKG_TARGET_IS_WINDOWS)
REF e140dfc8668e96d7e56cbd46467945adcc6b3cc4 #v6.2.0
Copy link
Contributor

Choose a reason for hiding this comment

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

This is missing a port-version update.

Copy link
Member Author

Choose a reason for hiding this comment

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

That was intentional because there are no changes in the output but...

@BillyONeal BillyONeal merged commit 1b1c17d into microsoft:master Oct 28, 2020
@BillyONeal BillyONeal deleted the build_yasm branch October 28, 2020 21:26
arthurt added a commit to arthurt/libsndfile that referenced this pull request Nov 30, 2020
arthurt added a commit to arthurt/libsndfile that referenced this pull request Nov 30, 2020
arthurt added a commit to arthurt/libsndfile that referenced this pull request Dec 21, 2020
arthurt added a commit to arthurt/libsndfile that referenced this pull request Jan 4, 2021
arthurt added a commit to arthurt/libsndfile that referenced this pull request Jan 23, 2021
arthurt added a commit to arthurt/libsndfile that referenced this pull request Mar 22, 2021
arthurt added a commit to libsndfile/libsndfile that referenced this pull request Mar 25, 2021
strega-nil pushed a commit to strega-nil/vcpkg that referenced this pull request May 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[gmp:x64-windows] build failure
5 participants