Skip to content
This repository has been archived by the owner on Jul 4, 2023. It is now read-only.

swiftgen 0.5.0 fix #45008

Closed
wants to merge 1 commit into from
Closed

swiftgen 0.5.0 fix #45008

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion Library/Formula/swiftgen.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,20 @@ class Swiftgen < Formula
url "https://github.com/AliSoftware/SwiftGen/archive/0.5.0.tar.gz"
sha256 "555f190f2ffef940eebd80a926eeb05d3d0de573412028c5bd2184e2b9542929"
head "https://github.com/AliSoftware/SwiftGen.git"
revision 1

bottle do
cellar :any
sha256 "29391f183e1606e50008ad148b3665d240c28ab5cbd42a293d1ca99b29aa3793" => :el_capitan
sha256 "c150775bf2f6b8e77a821eae7bd89233a134eb22ce5eed88de9eaf2822dd79ee" => :yosemite
end

def pour_bottle?
Copy link
Member

Choose a reason for hiding this comment

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

This is too harsh, IMO. Rather than banning bottles for everyone it would seem to make more sense to check whether Xcode was installed in the default location. Honestly I'd be surprised if there are many people not running Xcode out of /Applications though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are a LOT of people having Xcode in /Applications but not named Xcode.app.

Especially now as people (like me) tend to keep an Xcode6.app around for some time for legacy projects, also have Xcode7.app for everyday projects, and also have Xcode7.1beta.app for testing tvOS

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is there any guarantee that the machine BrewBot uses to build the bottle use an Xcode named /Applications.Xcode.app all the time? In that case I can indeed only use the bottle if it's also the case for the user.

Copy link
Member

Choose a reason for hiding this comment

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

Given a not tiny number of compile scripts specifically check for /Applications/Xcode.app and presume you don't have one unless it is named exactly that, having no Xcode.app is quite risky. You can keep multiple Xcodes, but you'll hit significantly fewer issues if one is kept as the default name.

Copy link
Member

Choose a reason for hiding this comment

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

Is there any guarantee that the machine BrewBot uses to build the bottle use an Xcode named /Applications.Xcode.app all the time?

On stable OS X versions, yes. On very beta OS X versions it may well be Xcode-beta.app but 10.9, 10.10 & 10.11 are all stable. The first runs Xcode 6.4, the latter two run 7.0.1, all three are /Applications/Xcode.app.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Given a not tiny number of compile scripts specifically check for /Applications/Xcode.app and presume you don't have one unless it is named exactly that, having no Xcode.app is quite risky. You can keep multiple Xcodes, but you'll hit significantly fewer issues if one is kept as the default name.

Sure, but:

  • those scripts are being bad citizens, and I prefer to be a good one
  • that's not always anybody's choice.

For example in the case of the OP of SwiftGen/SwiftGen#35 (comment) the issue is on CircleCI, not even his own machine. So I think I should still ensure it works on such contexts.

Copy link
Member

Choose a reason for hiding this comment

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

Of course. Not saying you can't or wouldn't want to work on a fix, just saying that banning everyone from using bottles is probably a bit too broad in this particular situation when we can do a check to see where Xcode is installed and pour if true.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Totally agreed, and just fixed. Thx!

# The binary's @rpath points to Xcode.app internal dylibs, so using a bottle won't work if the user doesn't
# have an Xcode installed in /Applications/Xcode.app (= the path used when BrewBot built the bottle)
Pathname.new("/Applications/Xcode.app").exist?
end

depends_on :xcode => "7.0"

def install
Expand All @@ -30,7 +37,7 @@ def install
end

test do
system "#{bin}/swiftgen --version"
system bin/"swiftgen", "--version"

output = shell_output("#{bin}/swiftgen images #{pkgshare}/Images.xcassets").strip
assert_equal output, (pkgshare/"Images-File-Defaults.swift.out").read.strip, "swiftgen images failed"
Expand Down