fix system image library uninstallation on macos x86 #7622

pmeier · 2023-05-24T10:00:35Z

#7417 touched the general dev env creation for CI. Amongst other things, it now also install system libpng libraries on macOS x86. However, this leads to failures with our I/O tests: https://github.com/pytorch/vision/actions/runs/5057898641/jobs/9077254208. This uninstallation was part of the journey to get CMake running, but was no longer needed in the end.

Thanks to @NicolasHug observation, it turned out that the root cause was a broken uninstall logic and nothing to do with the PNG libs. This PR now fixes the error described in #7622 (comment).

cc @seemethere

pytorch-bot · 2023-05-24T10:00:42Z

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/vision/7622

📄 Preview Python docs built from this PR

Note: Links to docs will display an error until the docs builds have been completed.

❌ 32 New Failures

As of commit 01c1a95:

NEW FAILURES - The following jobs have failed:

This comment was automatically generated by Dr. CI and updates every 15 minutes.

pmeier · 2023-05-24T10:25:27Z

8c5b877 has a green CI for all CMake and unittest workflows.

This reverts commit f30e413.

NicolasHug

Stamping, but why is removing the uninstallation of png lib has an effect on the jpeg lib? IIUC the failures in https://pipelines.actions.githubusercontent.com/serviceHosts/e1cea170-7cad-4fd4-b94c-3e3f4452f685/_apis/pipelines/1/runs/60504/signedlogcontent/24?urlExpires=2023-05-24T10%3A29%3A04.2815900Z&urlSigningMethod=HMACV1&urlSignature=k7F0EHX1UgrEUKv1kqdLfWJGDUDX7mK%2Bx543dpy3418%3D relate to libjpeg, not libpng

pmeier · 2023-05-24T10:36:24Z

That is a really good question. I honestly didn't see that. My guess is that the new logic is somehow wrong and jpeg library is not uninstalled for some reason. Let me check.

pmeier · 2023-05-24T10:42:35Z

Log output

before:

2023-05-23T07:05:30.8266370Z ##[group]Uninstall system JPEG libraries on macOS
2023-05-23T07:05:43.3191720Z jpeg-turbo jpeg-xl openjpeg
2023-05-23T07:05:46.3387080Z Uninstalling jpeg-turbo... (44 files, 3.9MB)
2023-05-23T07:05:49.6651010Z Uninstalling jpeg-xl... (43 files, 19.4MB)
2023-05-23T07:05:52.1431560Z Uninstalling openjpeg... (536 files, 13.8MB)
2023-05-23T07:05:52.6152150Z ##[endgroup]

So I guess I'm right since the log after the patch never has the Uninstalling ... output coming from brew.

after:

2023-05-23T13:28:30.1339840Z + echo '::group::Uninstall system JPEG libraries on macOS'
2023-05-23T13:28:30.1340560Z ++ brew list
2023-05-23T13:28:30.1389040Z ++ grep -E 'jpeg|png'
2023-05-23T13:28:44.4600320Z + IMAGE_LIBS='jpeg-turbo
2023-05-23T13:28:44.4600770Z jpeg-turbo
2023-05-23T13:28:44.4697740Z jpeg-xl
2023-05-23T13:28:44.4698360Z jpeg-xl
2023-05-23T13:28:44.4732740Z libpng
2023-05-23T13:28:44.4733290Z libpng
2023-05-23T13:28:44.4790080Z openjpeg'
2023-05-23T13:28:44.4790790Z openjpeg
2023-05-23T13:28:44.4822750Z + echo 'jpeg-turbo
2023-05-23T13:28:44.4838960Z jpeg-xl
2023-05-23T13:28:44.4885750Z libpng
2023-05-23T13:28:44.4924170Z openjpeg'
2023-05-23T13:28:44.4970110Z + for lib in '"${IMAGE_LIBS}"'
2023-05-23T13:28:44.5014900Z + brew uninstall --ignore-dependencies --force 'jpeg-turbo
2023-05-23T13:28:44.5066140Z jpeg-xl
2023-05-23T13:28:44.5107200Z libpng
2023-05-23T13:28:44.5131170Z openjpeg'
2023-05-23T13:28:47.4642280Z + echo ::endgroup::

This reverts commit 8c5b877.

pmeier · 2023-05-24T11:04:13Z

Ok, there were two things coming together with

  for lib in "${IMAGE_LIBS}"; do
    brew uninstall --ignore-dependencies --force "${lib}" || true

I botched the loop by wrapping IMAGE_LIBS into a string. Thus, instead of iterating over the individual libs, we just had one string. For example:

LIBS="foo bar baz"

echo 1
for lib in $LIBS; do
    echo "${lib}"
done

echo 2
for lib in "${LIBS}"; do
    echo "${lib}"
done

Running this prints

1
foo
bar
baz
2
foo bar baz

While definitely a bug, this went undetected because we also ignored the error code brew uninstall ... || true.

I've fixed both things in 334b4e5

pmeier · 2023-05-24T11:13:16Z

Looking good now

  + echo '::group::Uninstall system JPEG libraries on macOS'
  ++ brew list
  ++ grep -E 'jpeg|png'
  + IMAGE_LIBS='jpeg-turbo
  jpeg-xl
  libpng
  openjpeg'
  + for lib in '$IMAGE_LIBS'
  + brew uninstall --ignore-dependencies --force jpeg-turbo
  Uninstalling jpeg-turbo... (44 files, 3.9MB)
  + for lib in '$IMAGE_LIBS'
  + brew uninstall --ignore-dependencies --force jpeg-xl
  Uninstalling jpeg-xl... (43 files, 19.4MB)
  + for lib in '$IMAGE_LIBS'
  + brew uninstall --ignore-dependencies --force libpng
  Uninstalling libpng... (27 files, 1.3MB)
  + for lib in '$IMAGE_LIBS'
  + brew uninstall --ignore-dependencies --force openjpeg
  Uninstalling openjpeg... (536 files, 13.8MB)
  + echo ::endgroup::

This reverts commit 48b25c2.

NicolasHug

Thanks!

pmeier · 2023-05-24T13:10:49Z

CMake and unittest jobs on macOS x86 are green!

github-actions · 2023-05-24T13:11:55Z

Hey @pmeier!

You merged this PR, but no labels were added. The list of valid labels is available at https://github.com/pytorch/vision/blob/main/.github/process_commit.py

Reviewed By: vmoens Differential Revision: D46314044 fbshipit-source-id: 64a92c371c3ccf3927169c9637c346302d3111a4

pmeier added 2 commits May 24, 2023 11:58

[DEBUG] try revert linpng uninstallation on macos-12

8c5b877

[REVERTME] disable unrelated CI

f30e413

facebook-github-bot added the cla signed label May 24, 2023

pmeier changed the title ~~[DEBUG] try revert linpng uninstallation on macos-12~~ try revert linpng uninstallation on macos-12 May 24, 2023

pmeier changed the title ~~try revert linpng uninstallation on macos-12~~ revert linpng uninstallation on macos-12 May 24, 2023

pmeier requested a review from NicolasHug May 24, 2023 10:25

pmeier added the module: ci label May 24, 2023

pmeier marked this pull request as ready for review May 24, 2023 10:25

pmeier added 2 commits May 24, 2023 12:25

Revert "[REVERTME] disable unrelated CI"

09295b3

This reverts commit f30e413.

Merge branch 'main' into macos-build

79168b4

NicolasHug approved these changes May 24, 2023

View reviewed changes

pmeier added 2 commits May 24, 2023 12:48

Revert "[DEBUG] try revert linpng uninstallation on macos-12"

f033fac

This reverts commit 8c5b877.

fix bash loop

334b4e5

pmeier marked this pull request as draft May 24, 2023 11:04

[REVERTME] disable unrelated CI

48b25c2

pmeier force-pushed the macos-build branch from aad265e to 48b25c2 Compare May 24, 2023 11:05

pmeier changed the title ~~revert linpng uninstallation on macos-12~~ fix system image library uninstallation on macos x86 May 24, 2023

Revert "[REVERTME] disable unrelated CI"

d7371d4

This reverts commit 48b25c2.

pmeier marked this pull request as ready for review May 24, 2023 11:14

NicolasHug approved these changes May 24, 2023

View reviewed changes

Merge branch 'main' into macos-build

01c1a95

pmeier merged commit 285500d into pytorch:main May 24, 2023

pmeier deleted the macos-build branch May 24, 2023 13:11

facebook-github-bot pushed a commit that referenced this pull request May 31, 2023

[fbsync] fix system image library uninstallation on macos x86 (#7622)

5efc10b

Reviewed By: vmoens Differential Revision: D46314044 fbshipit-source-id: 64a92c371c3ccf3927169c9637c346302d3111a4

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix system image library uninstallation on macos x86 #7622

fix system image library uninstallation on macos x86 #7622

pmeier commented May 24, 2023 •

edited

Loading

pytorch-bot bot commented May 24, 2023 •

edited

Loading

pmeier commented May 24, 2023

NicolasHug left a comment

pmeier commented May 24, 2023

pmeier commented May 24, 2023

pmeier commented May 24, 2023

pmeier commented May 24, 2023

NicolasHug left a comment

pmeier commented May 24, 2023

github-actions bot commented May 24, 2023

fix system image library uninstallation on macos x86 #7622

fix system image library uninstallation on macos x86 #7622

Conversation

pmeier commented May 24, 2023 • edited Loading

pytorch-bot bot commented May 24, 2023 • edited Loading

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/vision/7622

❌ 32 New Failures

pmeier commented May 24, 2023

NicolasHug left a comment

Choose a reason for hiding this comment

pmeier commented May 24, 2023

pmeier commented May 24, 2023

pmeier commented May 24, 2023

pmeier commented May 24, 2023

NicolasHug left a comment

Choose a reason for hiding this comment

pmeier commented May 24, 2023

github-actions bot commented May 24, 2023

pmeier commented May 24, 2023 •

edited

Loading

pytorch-bot bot commented May 24, 2023 •

edited

Loading