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

[libxcrypt] Add build requirements #38376

Merged

Conversation

WangWeiLin-MV
Copy link
Contributor

@WangWeiLin-MV WangWeiLin-MV commented Apr 24, 2024

Fix #38372

Add message of build requirements autoconf automake libtool pkg-config from upstream README

Checklist

  • Changes comply with the maintainer guide.
  • SHA512s are updated for each updated download.
  • The "supports" clause reflects platforms that may be fixed by this new version.
  • Any fixed CI baseline entries are removed from that file.
  • Any patches that are no longer applied are deleted from the port's directory.
  • The version database is fixed by rerunning ./vcpkg x-add-version --all and committing the result.
  • Only one version is added to each modified port's versions file.

Test

Port install tests pass with following triplets:

  • x64-linux

@WangWeiLin-MV WangWeiLin-MV added category:port-bug The issue is with a library, which is something the port should already support info:internal This PR or Issue was filed by the vcpkg team. labels Apr 24, 2024
@WangWeiLin-MV WangWeiLin-MV added category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist and removed category:port-bug The issue is with a library, which is something the port should already support labels Apr 24, 2024
@WangWeiLin-MV WangWeiLin-MV marked this pull request as ready for review April 24, 2024 07:31
@jimwang118 jimwang118 added the info:reviewed Pull Request changes follow basic guidelines label Apr 24, 2024
@JavierMatosD
Copy link
Contributor

I was not able to repro the original issue. I find it unlikely that this is the solution to the original problem.

@WangWeiLin-MV
Copy link
Contributor Author

I was not able to repro the original issue. I find it unlikely that this is the solution to the original problem.

@JavierMatosD Here is the bash script to repro the original issue.

podman build -t vcpkg-libxcrypt -f - <<EOF; podman run --rm -it vcpkg-libxcrypt sh reproduce1.sh
FROM ubuntu:latest

ARG DEBIAN_FRONTEND=noninteractive

RUN apt-get update && apt-get install -y \
  curl \
  g++ \
  git \
  make \
  unzip \
  zip \
  autoconf \
  automake \
  # libtool \
  pkg-config \
  && rm -rf /var/lib/apt/lists/*

ENV PATH /vcpkg:$PATH

RUN git clone https://github.com/microsoft/vcpkg \
  && cd /vcpkg \
  && bootstrap-vcpkg.sh

RUN bash -c 'echo -e "\
vcpkg install libxcrypt;\n\
cat /vcpkg/buildtrees/libxcrypt/autoconf-x64-linux-err.log;\n\
"' > reproduce1.sh

RUN bash -c 'echo -e "\
apt-get install -y libtool;\n\
vcpkg install libxcrypt;\n\
"' > reproduce2.sh

CMD ["/usr/bin/bash"]

EOF

# podman rmi vcpkg-libxcrypt:latest

@WangWeiLin-MV WangWeiLin-MV reopened this Apr 28, 2024
@JavierMatosD
Copy link
Contributor

I was not able to repro the original issue. I find it unlikely that this is the solution to the original problem.

@JavierMatosD Here is the bash script to repro the original issue.

podman build -t vcpkg-libxcrypt -f - <<EOF; podman run --rm -it vcpkg-libxcrypt sh reproduce1.sh
FROM ubuntu:latest

ARG DEBIAN_FRONTEND=noninteractive

RUN apt-get update && apt-get install -y \
  curl \
  g++ \
  git \
  make \
  unzip \
  zip \
  autoconf \
  automake \
  # libtool \
  pkg-config \
  && rm -rf /var/lib/apt/lists/*

ENV PATH /vcpkg:$PATH

RUN git clone https://github.com/microsoft/vcpkg \
  && cd /vcpkg \
  && bootstrap-vcpkg.sh

RUN bash -c 'echo -e "\
vcpkg install libxcrypt;\n\
cat /vcpkg/buildtrees/libxcrypt/autoconf-x64-linux-err.log;\n\
"' > reproduce1.sh

RUN bash -c 'echo -e "\
apt-get install -y libtool;\n\
vcpkg install libxcrypt;\n\
"' > reproduce2.sh

CMD ["/usr/bin/bash"]

EOF

# podman rmi vcpkg-libxcrypt:latest

Thank you! I'll check this out :)

Copy link
Contributor

@JavierMatosD JavierMatosD left a comment

Choose a reason for hiding this comment

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

Apologies @WangWeiLin-MV, I failed to realize that the original issue is on linux. I was originally skeptical because vcpkg_configure_make already prints a message and fails if it doesn't find autoconf. But it seems the script only handle automake, libtool, and pkg-config on windows. I think these changes are good to go :)

Sorry for my original review and thanks for the work!

@data-queue data-queue merged commit 9f22b3d into microsoft:master Apr 30, 2024
18 checks passed
@dg0yt
Copy link
Contributor

dg0yt commented Apr 30, 2024

I failed to realize that the original issue is on linux. I was originally skeptical because vcpkg_configure_make already prints a message and fails if it doesn't find autoconf. But it seems the script only handle automake, libtool, and pkg-config on windows. I think these changes are good to go :)

In fact it is a shame that we pollute all ports with redundant instructions for Ubuntu, Fedora, Alpine, macOS ... while everything is in one place for Windows.

@WangWeiLin-MV WangWeiLin-MV deleted the port/libxcrypt/requirements-message branch May 6, 2024 02:17
yurybura pushed a commit to yurybura/vcpkg that referenced this pull request May 8, 2024
Fix microsoft#38372

Add message of build requirements `autoconf` `automake` `libtool`
`pkg-config` from [upstream
README](https://github.com/besser82/libxcrypt?tab=readme-ov-file#build-requirements-and-instructions)

### Checklist
- [x] Changes comply with the [maintainer
guide](https://github.com/microsoft/vcpkg-docs/blob/main/vcpkg/contributing/maintainer-guide.md).
- [ ] ~SHA512s are updated for each updated download.~
- [ ] ~The "supports" clause reflects platforms that may be fixed by
this new version.~
- [ ] ~Any fixed [CI
baseline](https://github.com/microsoft/vcpkg/blob/master/scripts/ci.baseline.txt)
entries are removed from that file.~
- [ ] ~Any patches that are no longer applied are deleted from the
port's directory.~
- [x] The version database is fixed by rerunning `./vcpkg x-add-version
--all` and committing the result.
- [x] Only one version is added to each modified port's versions file.

### Test
Port install tests pass with following triplets:
* x64-linux
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 info:internal This PR or Issue was filed by the vcpkg team. info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[libxcrypt] Build error on x64-linux
5 participants