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

[python3][python2] Use MKDIR_P to create directories to avoid race conditions #22902

Merged
merged 3 commits into from
Feb 4, 2022

Conversation

ras0219-msft
Copy link
Contributor

@ras0219-msft ras0219-msft commented Feb 2, 2022

We've observed intermittent failures for python2 and python3 on osx with the error log

install: mkdir /Users/vagrant/Data/packages/python3_x64-osx/Users/vagrant/Data/installed/x64-osx/lib: File exists
make: *** [altbininstall] Error 71

Looking at the implementation of altbininstall, it is using $(INSTALL) to create these folders. Apprently, the /usr/bin/install program on osx does not internally pass -p to mkdir under normal circumstances. However, autotools already has provided for this by finding an appropriate mkdir that's safe:

# from config.log
configure:6351: checking for a thread-safe mkdir -p
configure:6390: result: ./../src/v3.10.1-8b5dfc5615.clean/install-sh -c -d

This program is stored as MKDIR_P, so I've added a string replacement step that changes over every mkdir-via-install to use $(MKDIR_P) instead. I originally wanted to use a patch, but there are around 10 uses scattered through the file so it would be a reasonably large patchfile (~100 lines).

There is potentially a concern about the produced directories not having customized permissions, however I don't think that should be an issue in the case of vcpkg.

@ras0219-msft ras0219-msft changed the title [python3][python2] Use mkdir -p to create directories to avoid race conditions [python3][python2] Use MKDIR_P to create directories to avoid race conditions Feb 2, 2022
@strega-nil
Copy link
Contributor

LGTM!

@dg0yt
Copy link
Contributor

dg0yt commented Feb 2, 2022

For python3, we already applied #21734. Maybe it should be reverted if it wasn't sufficient.

@JackBoosY JackBoosY self-assigned this Feb 3, 2022
@JackBoosY JackBoosY 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. requires:author-response labels Feb 3, 2022
…icrosoft#21734)"

This reverts commit a515872.

# Conflicts:
#	ports/python3/vcpkg.json
#	versions/baseline.json
#	versions/p-/python3.json
@BillyONeal BillyONeal merged commit c0d667a into microsoft:master Feb 4, 2022
@BillyONeal
Copy link
Member

Thanks for the fix!

ekilmer added a commit to ekilmer/vcpkg that referenced this pull request Feb 5, 2022
* master: (221 commits)
  [vcpkg-tool] update to 2022-02-03 (microsoft#22924)
  [opencv4] Disable building cpufeatures since it conflict with libwebp (microsoft#22844)
  [rhasheq] New port (microsoft#22905)
  [sfml] Add arm64 patch to allow SFML to compile on apple silicon (microsoft#22937)
  [popsift] Fix missing Thrust include, already merged upstream. (microsoft#22929)
  [python3][python2] Use MKDIR_P to create directories to avoid race conditions (microsoft#22902)
  Added libe57format port (microsoft#22909)
  update polyhook2 (microsoft#22906)
  [botan] Fix debug info (microsoft#22911)
  [opentelemetry-cpp] update version to v1.2.0 (microsoft#22925)
  [docs] document VCPKG_INSTALLED_DIR variable (microsoft#22695)
  [c89stringutils] New port (microsoft#22904)
  [randomstr] New port (microsoft#22921)
  [docs] Add Authoring-script-ports.md (microsoft#22396)
  [vcpkg_fail_port_install] Deprecate function (microsoft#21489)
  [vcpkg-cmake-config] add missing TOOLS_PATH (microsoft#22863)
  Ace Build Windows - Missing files in include package (microsoft#22880)
  [opendnp3] Disable FetchContent in favor of predownloading (microsoft#22894)
  libraqm update to 0.9.0 (microsoft#22907)
  [google-cloud-cpp] update to latest release (v1.36.0) (microsoft#22897)
  ...
@ras0219-msft ras0219-msft deleted the dev/roschuma/python-osx branch February 10, 2022 18:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants