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

audit: avoid python symlink conflict #8959

Merged
merged 2 commits into from
Oct 20, 2020
Merged

audit: avoid python symlink conflict #8959

merged 2 commits into from
Oct 20, 2020

Conversation

Rylan12
Copy link
Member

@Rylan12 Rylan12 commented Oct 20, 2020

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same change?
  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes? Here's an example.
  • Have you successfully run brew style with your changes locally?
  • Have you successfully run brew tests with your changes locally?
  • Have you successfully run brew man locally and committed any changes?

Resolves #8949

Only allow one python formula to create pip3 and wheel3 symlinks

CC: @fxcoudert, @SMillerDev and @MikeMcQuaid

Sorry, something went wrong.

Copy link
Member

@MikeMcQuaid MikeMcQuaid left a comment

Choose a reason for hiding this comment

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

Thanks for this @Rylan12! I think this is a good test but I wonder if it's overly specific and we'd be better looking at the actually written symlinks in brew audit instead. Thoughts? ✅ because this is better than the status quo and we could consider having both approaches.

@Rylan12
Copy link
Member Author

Rylan12 commented Oct 20, 2020

I think the benefit of checking the actual symlinks in brew audit is it checks the actual links rather than the code that generates the links (which could change rendering this check pretty useless). So, going with an audit is probably a better solution. I'll look into it in about an hour when I've got some more time.

In terms of specificity, what would the less specific audit be? As far as I know, this is the only formula with this kind of symlink issue. The problem is inherently specific because we do want to allow some symlinks (e.g. pip3.x) but not all symlinks (e.g. pip3)

@MikeMcQuaid
Copy link
Member

In terms of specificity, what would the less specific audit be?

Sorry, to be clear, I thought this RuboCop was overly specific (due to the reasons you stated yourself) but any brew audit checking these symlinks would not be problematically specific 👍🏻

@Rylan12 Rylan12 changed the title style: avoid python symlink conflict audit: avoid python symlink conflict Oct 20, 2020
@Rylan12 Rylan12 requested a review from MikeMcQuaid October 20, 2020 16:09
@Rylan12 Rylan12 merged commit a6ee5b7 into Homebrew:master Oct 20, 2020
@Rylan12 Rylan12 deleted the python-symlink-rubocop branch October 20, 2020 19:47
@BrewTestBot BrewTestBot added the outdated PR was locked due to age label Dec 1, 2020
@Homebrew Homebrew locked as resolved and limited conversation to collaborators Dec 1, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated PR was locked due to age
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Audit to avoid conflict in python symlinks
4 participants