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

mermaid-cli 8.11.4 #83338

Closed
wants to merge 1 commit into from
Closed

mermaid-cli 8.11.4 #83338

wants to merge 1 commit into from

Conversation

chenrui333
Copy link
Member

Created with brew bump-formula-pr.

@BrewTestBot BrewTestBot added bump-formula-pr PR was created using `brew bump-formula-pr` nodejs Node or npm use is a significant feature of the PR or issue labels Aug 15, 2021
@chenrui333 chenrui333 added audit failure CI fails while auditing the software CI-no-fail-fast Continue CI tests despite failing GitHub Actions matrix builds. labels Aug 15, 2021
@chenrui333
Copy link
Member Author

chenrui333 commented Aug 15, 2021 •
edited

on 11-arm64

mermaid-cli:
  * Binaries built for a non-native architecture were installed into mermaid-cli's prefix.
    The offending files are:
      /opt/homebrew/Cellar/mermaid-cli/8.11.4/libexec/lib/node_modules/@mermaid-js/mermaid-cli/node_modules/puppeteer/.local-chromium/mac-901912/chrome-mac/Chromium.app/Contents/Frameworks/Chromium Framework.framework/Versions/93.0.4577.0/Chromium Framework	(x86_64)
      /opt/homebrew/Cellar/mermaid-cli/8.11.4/libexec/lib/node_modules/@mermaid-js/mermaid-cli/node_modules/puppeteer/.local-chromium/mac-901912/chrome-mac/Chromium.app/Contents/Frameworks/Chromium Framework.framework/Versions/93.0.4577.0/Helpers/Chromium Helper (Alerts).app/Contents/MacOS/Chromium Helper (Alerts)	(x86_64)
      /opt/homebrew/Cellar/mermaid-cli/8.11.4/libexec/lib/node_modules/@mermaid-js/mermaid-cli/node_modules/puppeteer/.local-chromium/mac-901912/chrome-mac/Chromium.app/Contents/Frameworks/Chromium Framework.framework/Versions/93.0.4577.0/Helpers/Chromium Helper (GPU).app/Contents/MacOS/Chromium Helper (GPU)	(x86_64)
      /opt/homebrew/Cellar/mermaid-cli/8.11.4/libexec/lib/node_modules/@mermaid-js/mermaid-cli/node_modules/puppeteer/.local-chromium/mac-901912/chrome-mac/Chromium.app/Contents/Frameworks/Chromium Framework.framework/Versions/93.0.4577.0/Helpers/Chromium Helper (Plugin).app/Contents/MacOS/Chromium Helper (Plugin)	(x86_64)
      /opt/homebrew/Cellar/mermaid-cli/8.11.4/libexec/lib/node_modules/@mermaid-js/mermaid-cli/node_modules/puppeteer/.local-chromium/mac-901912/chrome-mac/Chromium.app/Contents/Frameworks/Chromium Framework.framework/Versions/93.0.4577.0/Helpers/Chromium Helper (Renderer).app/Contents/MacOS/Chromium Helper (Renderer)	(x86_64)
      /opt/homebrew/Cellar/mermaid-cli/8.11.4/libexec/lib/node_modules/@mermaid-js/mermaid-cli/node_modules/puppeteer/.local-chromium/mac-901912/chrome-mac/Chromium.app/Contents/Frameworks/Chromium Framework.framework/Versions/93.0.4577.0/Helpers/Chromium Helper.app/Contents/MacOS/Chromium Helper	(x86_64)
      /opt/homebrew/Cellar/mermaid-cli/8.11.4/libexec/lib/node_modules/@mermaid-js/mermaid-cli/node_modules/puppeteer/.local-chromium/mac-901912/chrome-mac/Chromium.app/Contents/Frameworks/Chromium Framework.framework/Versions/93.0.4577.0/Helpers/app_mode_loader	(x86_64)
      /opt/homebrew/Cellar/mermaid-cli/8.11.4/libexec/lib/node_modules/@mermaid-js/mermaid-cli/node_modules/puppeteer/.local-chromium/mac-901912/chrome-mac/Chromium.app/Contents/Frameworks/Chromium Framework.framework/Versions/93.0.4577.0/Helpers/chrome_crashpad_handler	(x86_64)
      /opt/homebrew/Cellar/mermaid-cli/8.11.4/libexec/lib/node_modules/@mermaid-js/mermaid-cli/node_modules/puppeteer/.local-chromium/mac-901912/chrome-mac/Chromium.app/Contents/Frameworks/Chromium Framework.framework/Versions/93.0.4577.0/Libraries/libEGL.dylib	(x86_64)
      /opt/homebrew/Cellar/mermaid-cli/8.11.4/libexec/lib/node_modules/@mermaid-js/mermaid-cli/node_modules/puppeteer/.local-chromium/mac-901912/chrome-mac/Chromium.app/Contents/Frameworks/Chromium Framework.framework/Versions/93.0.4577.0/Libraries/libGLESv2.dylib	(x86_64)
      /opt/homebrew/Cellar/mermaid-cli/8.11.4/libexec/lib/node_modules/@mermaid-js/mermaid-cli/node_modules/puppeteer/.local-chromium/mac-901912/chrome-mac/Chromium.app/Contents/Frameworks/Chromium Framework.framework/Versions/93.0.4577.0/Libraries/libswiftshader_libEGL.dylib	(x86_64)
      /opt/homebrew/Cellar/mermaid-cli/8.11.4/libexec/lib/node_modules/@mermaid-js/mermaid-cli/node_modules/puppeteer/.local-chromium/mac-901912/chrome-mac/Chromium.app/Contents/Frameworks/Chromium Framework.framework/Versions/93.0.4577.0/Libraries/libswiftshader_libGLESv2.dylib	(x86_64)
      /opt/homebrew/Cellar/mermaid-cli/8.11.4/libexec/lib/node_modules/@mermaid-js/mermaid-cli/node_modules/puppeteer/.local-chromium/mac-901912/chrome-mac/Chromium.app/Contents/Frameworks/Chromium Framework.framework/Versions/93.0.4577.0/Libraries/libvk_swiftshader.dylib	(x86_64)
      /opt/homebrew/Cellar/mermaid-cli/8.11.4/libexec/lib/node_modules/@mermaid-js/mermaid-cli/node_modules/puppeteer/.local-chromium/mac-901912/chrome-mac/Chromium.app/Contents/Frameworks/Chromium Framework.framework/Versions/93.0.4577.0/XPCServices/AlertNotificationService.xpc/Contents/MacOS/AlertNotificationService	(x86_64)
      /opt/homebrew/Cellar/mermaid-cli/8.11.4/libexec/lib/node_modules/@mermaid-js/mermaid-cli/node_modules/puppeteer/.local-chromium/mac-901912/chrome-mac/Chromium.app/Contents/MacOS/Chromium	(x86_64)

failed to build on x86_64

@chenrui333 chenrui333 added the build failure CI fails while building the software label Aug 15, 2021
@github-actions
Copy link
Contributor

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. To keep this pull request open, add a help wanted or in progress label.

@github-actions github-actions bot added the stale No recent activity label Aug 18, 2021
@danielnachun danielnachun removed the stale No recent activity label Aug 18, 2021
@danielnachun
Copy link
Member

I suspected we'd hit this problem with mermaid-cli when I was testing this on Linux earlier this year. This is happening because it uses puppeteer to actually render the diagrams with a pre-built copy of Chromium, which is totally broken on Linux because it can't find some libraries, and now we're finding that it's downloading universal binaries too.

Apparently the reason puppeteer has to download its own copy of Chromium is because (unsurprisingly) there is poor API stability across Chromium versions and each release of puppeteer is tied to a very specific version of Chromium. Apparently we can set the environment variable PUPPETEER_SKIP_CHROMIUM_DOWNLOAD=1 to stop Chromium from being downloaded, but this would just require the user to download that copy of Chromium themselves instead, and would mean that the bottle we ship is not usable without this extra step.

Hypothetically that specific version of Chromium could be built from source, but I don't think we have any desire to do that. While we could make this macOS only and give it an audit exception for the universal binaries, I feel like that may just be a bandage and the reality is that a formula like this may not belong in homebrew-core. What do other maintainers think?

@SMillerDev
Copy link
Member

I think copying a browser into your install is just taking it too far. I'd be in favor of disabling it.

@danielnachun
Copy link
Member

I think copying a browser into your install is just taking it too far.

💯

On a more serious note the upstream repo does mention Homebrew as an install option, so if we decide to disable it maybe they should be notified? Fortunately I think macOS users would be fine installing it with npm (Linux might have issues), and they provide a Docker image as well, so users would have several other options to get the software.

In terms of popularity, it had 300 installs in the last 30 days. I'm not sure if that is considered "popular" on macOS, but those are numbers in case that helps with the decision.

@carlocab
Copy link
Member

This actually doesn't install any universal binaries -- it installs x86_64-only binaries on ARM.

The error text for universal binaries is Unexpected universal binaries were found while the error text we see is Binaries built for a non-native architecture were installed into mermaid-cli's prefix...

@SMillerDev
Copy link
Member

In terms of popularity, it had 300 installs in the last 30 days. I'm not sure if that is considered "popular" on macOS, but those are numbers in case that helps with the decision.

I'd say that's pretty popular, but downloading a browser is a really bad packaging decision. So in my mind the problems outweigh the benefits (since a tap is easy to maintain).

@SMillerDev SMillerDev requested a review from a team August 18, 2021 20:26
@SMillerDev SMillerDev added the maintainer feedback Additional maintainers' opinions may be needed label Aug 18, 2021
@alebcay
Copy link
Member

alebcay commented Aug 18, 2021 •
edited

I agree that bundling a browser feels like a bit much. Sadly there isn't really a good substitute for puppeteer in terms of what it provides (e.g. I've used it to scrape download links out of pages; with some single-page applications, you have to scroll down to get more content to load, etc. Not as simple as pointing a CLI scraper at it). With some config you can point puppeteer to use an existing browser binary rather than the provided one (they provide it as a convenience), but we can't make that assumption either.

Once upon a time there was a variant of headless variant of Chromium (not the same as running Chrome/Chromium with --headless) that was a lot smaller than Chromium proper but still scriptable. Unfortunately I don't know if they've kept up with providing that (it was only ever provided at a nightly/dev-level of support), not to mention that things probably only got more complicated with Apple Silicon. That's also not even considering Google's heavily customized gn build system and everything else used in the Chromium build process.

@danielnachun
Copy link
Member

The packaging situation with puppeteer is a perfect encapsulation of two huge problems:

  1. Developers who only know Javascript and refuse to learn how to do proper software packaging. TBH I'm seeing similar issues with maven so the JDK world is no better, and the way Python, Ruby and Perl have handled packaging that interfaces with native code is not great either. There are very good reasons why all established package managers want to build native binaries from source themselves, and I wish developers using these higher level languages took more time to understand those reasons.
  2. The refusal/inability of large companies like Google and Facebook to put serious effort into making their "open source" software easy to package, and to have those packages provide a reasonably stable API (hence why puppeteer has to use specific versions of Chromium). I get that the web-facing part of the browser needs frequent updates for security purposes, but there is no reason in my mind why a stable scripting API can't be created. If Chromium was developed in a more modular way with different components communicating with each other with stable, documented APIs, it would be both easier to package the software and to create external tools to use those APIs.

Thus far we've managed to hack around some of these issues by deleting upstream binaries, or figuring out how to build the binaries ourselves. But we have to draw the line somewhere, and this tool probably crosses it.

I'm in favor of telling the upstream repo here to make a macOS-only tap, and just direct their Linux users to use Docker. Distributing a binary build of Chromium that would work on the range of Linux configurations we support is a fruitless exercise.

@MikeMcQuaid MikeMcQuaid removed the request for review from a team August 19, 2021 10:23
@github-actions
Copy link
Contributor

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. To keep this pull request open, add a help wanted or in progress label.

@github-actions github-actions bot added the stale No recent activity label Aug 21, 2021
@github-actions github-actions bot closed this Aug 23, 2021
@cho-m cho-m mentioned this pull request Jan 24, 2022
Closed
@cho-m cho-m mentioned this pull request Feb 20, 2022
Closed
@nandahkrishna nandahkrishna mentioned this pull request Apr 26, 2022
Closed
2 tasks
@yogurtearl yogurtearl mentioned this pull request Apr 26, 2022
Closed
@chenrui333 chenrui333 deleted the bump-mermaid-cli-8.11.4 branch December 18, 2022 05:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
audit failure CI fails while auditing the software build failure CI fails while building the software bump-formula-pr PR was created using `brew bump-formula-pr` CI-no-fail-fast Continue CI tests despite failing GitHub Actions matrix builds. maintainer feedback Additional maintainers' opinions may be needed nodejs Node or npm use is a significant feature of the PR or issue stale No recent activity
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@chenrui333 @danielnachun @SMillerDev @carlocab @alebcay @BrewTestBot