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.

Sign up for GitHub

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

Jump to bottom

try fixing x264 #13450

Merged
merged 3 commits into from
Sep 24, 2020
Merged

try fixing x264 #13450

merged 3 commits into from
Sep 24, 2020

Conversation

Neumann-A
Copy link
Contributor

@Neumann-A Neumann-A commented Sep 10, 2020 •
edited

will close #13327 if it works.

Current problem:
install.exe is not installing for some reason

@ras0219 @JackBoosY @NancyLi1013 Can somebody figure out why msys install.exe is not installing?

The logs indicate that the following commands are run:


Makefile:381: update target 'install-cli' due to: cli
install -d /E/Repos/vcpkg/packages/x264_x64-windows-static/E/Repos/vcpkg/installed/x64-windows-static/debug/bin
Makefile:385: target 'install-lib-dev' does not exist
install -d /E/Repos/vcpkg/packages/x264_x64-windows-static/E/Repos/vcpkg/installed/x64-windows-static/debug/include /E/Repos/vcpkg/packages/x264_x64-windows-static/E/Repos/vcpkg/installed/x64-windows-static/debug/lib/pkgconfig
install x264.exe /E/Repos/vcpkg/packages/x264_x64-windows-static/E/Repos/vcpkg/installed/x64-windows-static/debug/bin
install -m 644 ./x264.h x264_config.h /E/Repos/vcpkg/packages/x264_x64-windows-static/E/Repos/vcpkg/installed/x64-windows-static/debug/include
install -m 644 x264.pc /E/Repos/vcpkg/packages/x264_x64-windows-static/E/Repos/vcpkg/installed/x64-windows-static/debug/lib/pkgconfig
Makefile:390: update target 'install-lib-static' due to: lib-static install-lib-dev
install -d /E/Repos/vcpkg/packages/x264_x64-windows-static/E/Repos/vcpkg/installed/x64-windows-static/debug/lib
install -m 644 libx264.lib /E/Repos/vcpkg/packages/x264_x64-windows-static/E/Repos/vcpkg/installed/x64-windows-static/debug/lib

but the target directories are not created / empty

Hmm i found everything in
E:\vcpkg_cache\downloads\tools\msys2\77f3fd5f29257d6b\E\Repos\vcpkg\packages\x264_x64-windows-static\E\Repos\vcpkg

@JackBoosY JackBoosY self-assigned this Sep 10, 2020
@JackBoosY JackBoosY added category:port-bug The issue is with a library, which is something the port should already support category:tool-update The issue is with build tool or build script, which requires update or should be executed correctly labels Sep 10, 2020
JackBoosY
JackBoosY suggested changes Sep 10, 2020
View reviewed changes
Copy link
Contributor

@JackBoosY JackBoosY left a comment

Choose a reason for hiding this comment

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

Bump version please.

@Neumann-A
Copy link
Contributor Author

@JackBoosY: Remind me if it is out of draft and i didn't bump it..... until then I'll focus on the bigger picture instead of minor less important details.

Neumann-A
Neumann-A commented Sep 10, 2020
View reviewed changes
scripts/cmake/vcpkg_configure_make.cmake
@@ -243,8 +240,14 @@ function(vcpkg_configure_make)
endif()
endif()
debug_message("Using make triplet: ${_csc_BUILD_TRIPLET}")
set(APPEND_ENV ";${MSYS_ROOT}/usr/share/automake-1.16")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

unsure if this should be before /usr/bin or after. The problem here is that /usr/share/automake-1.16 contains an install script which conflicts with install.exe in /usr/bin

Copy link
Contributor

Choose a reason for hiding this comment

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

It should be good.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't quite follow how this PR fixes things; it looks like automake will still come before /usr/bin and thus will still mask install?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

X264 is not a autoconf port but some custom configure. As such the path with the script is not added to the path variable for the port.

Neumann-A
Neumann-A commented Sep 10, 2020
View reviewed changes
ports/x264/portfile.cmake Outdated Show resolved Hide resolved
@JackBoosY
Copy link
Contributor

Looks passed.

@NancyLi1013
Copy link
Contributor

Seems also need to update ci.baseline.txt.

@Neumann-A Neumann-A marked this pull request as ready for review September 11, 2020 08:40
JackBoosY
JackBoosY approved these changes Sep 14, 2020
View reviewed changes
scripts/cmake/vcpkg_configure_make.cmake
@@ -243,8 +240,14 @@ function(vcpkg_configure_make)
endif()
endif()
debug_message("Using make triplet: ${_csc_BUILD_TRIPLET}")
set(APPEND_ENV ";${MSYS_ROOT}/usr/share/automake-1.16")
Copy link
Contributor

Choose a reason for hiding this comment

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

It should be good.

@JackBoosY JackBoosY added the info:reviewed Pull Request changes follow basic guidelines label Sep 14, 2020
This was referenced Sep 16, 2020
Closed
Closed
This was referenced Sep 21, 2020
Closed
Closed
ras0219-msft
ras0219-msft reviewed Sep 24, 2020
View reviewed changes
scripts/cmake/vcpkg_configure_make.cmake
endif()

string(REPLACE ";$ENV{SystemRoot}\\System32;" "${APPEND_ENV};${MSYS_ROOT}/usr/bin;$ENV{SystemRoot}\\System32;" NEWPATH "$ENV{PATH}")
string(REPLACE ";$ENV{SystemRoot}\\system32;" "${APPEND_ENV};${MSYS_ROOT}/usr/bin;$ENV{SystemRoot}\\system32;" NEWPATH "$ENV{PATH}")
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this duplicated twice?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is not duplicated. Since windows is case insensitive the path can be both ways. I tried a regex with [Ss] and escaping of the \ but cmake never matched it for some reason

@ras0219-msft
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@ras0219-msft ras0219-msft merged commit 2b02b97 into microsoft:master Sep 24, 2020
@ras0219-msft
Copy link
Contributor

Awesome, thanks!

@ras0219-msft ras0219-msft mentioned this pull request Sep 24, 2020
Closed
@Neumann-A Neumann-A deleted the fix_x264 branch September 24, 2020 19:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ras0219-msft ras0219-msft ras0219-msft approved these changes

@JackBoosY JackBoosY JackBoosY approved these changes

Assignees

@JackBoosY JackBoosY

Labels
category:port-bug The issue is with a library, which is something the port should already support category:tool-update The issue is with build tool or build script, which requires update or should be executed correctly info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[x264:x64-windows] build failure
4 participants
@Neumann-A @JackBoosY @NancyLi1013 @ras0219-msft