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

formula: add :swift dependency #7788

Closed
wants to merge 1 commit into from

Conversation

vladimyr
Copy link
Contributor

  • 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?

Enable Swift formulas to depend on :swift requirement:

depends_on :swift => ["5.2", :build]  

which on macOS translates into following:

depends_on :xcode => ["11.5", :build]

It simplifies the process of formula creation for Swift tools by automatic translation between Swift and Xcode versions. Previously authors were required to do manual translation and list resolved Xcode version as a requirement.

@vladimyr vladimyr mentioned this pull request Jun 22, 2020
6 tasks
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.

Good idea! Like where this is going so far.

@xcode_requirement.satisfied?
end

def xcode_required_version
Copy link
Member

Choose a reason for hiding this comment

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

Might be nice to keep this in Library/Homebrew/os/mac/xcode.rb so it's updated when that is.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually I put it there initially. I moved it out because this goes opposite direction which would make it only reverse lookup there but if you are ok with that I'll put it back 🤷

Copy link
Member

Choose a reason for hiding this comment

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

I think it's fine for the lookup to be the same way around as here but it's more likely to get bumped there, I think.

attr_reader :version

def initialize(tags = [])
@version = tags.shift if tags.first.to_s.match?(/(\d\.)+\d/)
Copy link
Member

Choose a reason for hiding this comment

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

Should this use the formula on Linux? Does it work there?

@bayandin
Copy link
Member

The only doubt I have that it's easy to confuse with swift formula.

@Bo98
Copy link
Member

Bo98 commented Jun 22, 2020

I could see this adapted to support the Swift formula on Linux at some point. Though first step there would be making the Swift formula work on Linux.

@vladimyr
Copy link
Contributor Author

Should this use the formula on Linux? Does it work there?

I could see this adapted to support the Swift formula on Linux at some point. Though first step there would be making the Swift formula work on Linux.

@Bo98 pretty much echoed my thoughts. I figured out by reading swift formula it needs some adjustments to get to the working state on Linux but unfortunately, I can't help with that one cause I don't have Linux machine to play with. I just wanted to make the first step...

@MikeMcQuaid
Copy link
Member

@vladimyr Fine for this to not work on Linux for now if the formula doesn't but might be nice to make this either try to use the formula on Linux or fail unconditionally without it.

@stale
Copy link

stale bot commented Jul 18, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale bot added the stale No recent activity label Jul 18, 2020
@stale stale bot closed this Jul 25, 2020
@BrewTestBot BrewTestBot added the outdated PR was locked due to age label Dec 22, 2020
@Homebrew Homebrew locked as resolved and limited conversation to collaborators Dec 22, 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 stale No recent activity
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants