Skip to content

pyside 6.2.1#89084

Closed
paperchalice wants to merge 1 commit intoHomebrew:masterfrom
paperchalice:pyside
Closed

pyside 6.2.1#89084
paperchalice wants to merge 1 commit intoHomebrew:masterfrom
paperchalice:pyside

Conversation

@paperchalice
Copy link
Copy Markdown
Contributor

  • Have you followed the guidelines for contributing?
  • Have you ensured that your commits follow the commit style guide?
  • Have you checked that there aren't other open pull requests for the same formula update/change?
  • Have you built your formula locally with brew install --build-from-source <formula>, where <formula> is the name of the formula you're submitting?
  • Is your test running fine brew test <formula>, where <formula> is the name of the formula you're submitting?
  • Does your build pass brew audit --strict <formula> (after doing brew install --build-from-source <formula>)? If this is a new formula, does it pass brew audit --new <formula>?

@BrewTestBot BrewTestBot added no Linux bottle Formula has no Linux bottle python Python use is a significant feature of the PR or issue labels Nov 10, 2021
@carlocab
Copy link
Copy Markdown
Member

Fix in Homebrew/homebrew-test-bot#695.

@carlocab carlocab added the CI-long-timeout [DEPRECATED] Use longer GitHub Actions CI timeout. label Nov 10, 2021
Comment on lines 60 to 61
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This looks wrong. Why does it still have the build prefix as the RPATH?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Because here uses the setup.py script, from official README, cmake super project is for development convenience. setup.py sets CMAKE_INSTALL_PREFIX to buildpath/pyside3_build/p3.9, so does the rpath, official shiboken wheel also sets the rpath to the build path. In order to keep the directory layout same as previous pyside version, which is the test case expect , we need manually move these files...

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If upstream have changed the directory layout then we should probably just follow them.

@paperchalice paperchalice force-pushed the pyside branch 4 times, most recently from d71c498 to a9790fb Compare November 11, 2021 03:46
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This issue has been reported to upstream.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can you link to the upstream report?

Comment on lines 48 to 51
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

install_symlink to a file in another keg will create a link that references a Cellar path. We don't want that because it breaks on upgrades.

Use ln_s instead. See julia, include-what-you-use, or any formulae that depends on macos-term-size for examples.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

use (qt.opt_bin/x).relative_path_from(site_pyside/x).

@carlocab
Copy link
Copy Markdown
Member

Thanks, @paperchalice. Can we switch this to llvm@12 too, for now? This would allow us to migrate llvm's and qt's dependency trees to python@3.10 separately.

Copy link
Copy Markdown
Member

@carlocab carlocab left a comment

Choose a reason for hiding this comment

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

Actually, never mind. I think there's a way to do it without moving pyside back to llvm@12.

@BrewTestBot
Copy link
Copy Markdown
Contributor

🤖 A scheduled task has triggered a merge.

@paperchalice paperchalice deleted the pyside branch November 28, 2021 06:05
@github-actions github-actions bot added the outdated PR was locked due to age label Dec 29, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 29, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

CI-long-timeout [DEPRECATED] Use longer GitHub Actions CI timeout. no Linux bottle Formula has no Linux bottle outdated PR was locked due to age python Python use is a significant feature of the PR or issue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants