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

Let flutter be installable via homebrew #14050

Open
xster opened this Issue Jan 11, 2018 · 19 comments

Comments

Projects
None yet
7 participants
@xster
Contributor

xster commented Jan 11, 2018

Didn't find any existing issues on this. Opening for tracking.

Let flutter be brew install flutterable.

@xster xster changed the title from Let flutter be installable via homebrew to Let flutter be installable via homebrew/apt-get Jan 11, 2018

@xster

This comment has been minimized.

Contributor

xster commented Jan 30, 2018

@gspencergoog you might be interested

@xster

This comment has been minimized.

Contributor

xster commented Mar 15, 2018

https://docs.brew.sh/Formula-Cookbook

The process is pretty straight-forward. Though we'd have to make sure it integrates with our https://github.com/flutter/flutter/wiki/Release-process, ideally in an automated way.

@xster xster added this to the 4: Next milestone milestone Mar 15, 2018

@Hixie Hixie added team and removed team: infra labels Apr 23, 2018

@briankung briankung referenced this issue May 2, 2018

Closed

flutter 0.3.1 (new formula) #27395

3 of 4 tasks complete
@briankung

This comment has been minimized.

briankung commented May 2, 2018

Hi Flutter team! I started adding a flutter formula to Homebrew at Homebrew/homebrew-core#27395, but I could definitely use more guidance on following the direction in this comment:

Though we'd have to make sure it integrates with our https://github.com/flutter/flutter/wiki/Release-process, ideally in an automated way.

Right now the Homebrew formula just downloads the current beta release, unzips it, and copies it to Flutter's Keg (eg. /usr/local/Cellar/flutter/0.3.1/).

Any feedback would be welcome!

@xster

This comment has been minimized.

Contributor

xster commented May 2, 2018

@jcollins-g wrt to the shlock from Homebrew/homebrew-core#27395, maybe we can check if shlock returns not permitted / 1 and parent process id is brew, don't shlock. Also we can lock write to parts of the cache to prevent self update when brew installed.

@briankung

This comment has been minimized.

briankung commented May 2, 2018

For context in that Homebrew PR, I wrote about the shlock issue in the comments starting here: https://github.com/Homebrew/homebrew-core/pull/27395/files#diff-b438c13e924f5f36b1ce4398a89436d2R36

@xster

This comment has been minimized.

Contributor

xster commented May 2, 2018

Actually, we went through this idea a bit more and I think ultimately, we'd want the official formula to be in a homebrew tap (non-core repository). This way, we can keep the flutter upgrade and brew upgrade targets hermetically synchronized.

Hopefully we can bring a version of your formula into the tap in the near future and simplify the official install process.

@briankung

This comment has been minimized.

briankung commented May 2, 2018

Sure! In that case, I'd be happy to close the PR on homebrew-core if that makes sense?

@xster

This comment has been minimized.

Contributor

xster commented May 3, 2018

Yes. Thanks for making the contribution!

@ilovezfs

This comment has been minimized.

ilovezfs commented May 3, 2018

@xster fwiw that's not necessary. Formulae in Homebrew/homebew-core get upgraded within hours of upstream releases since we run brew livecheck several times per day.

@jcollins-g

This comment has been minimized.

Contributor

jcollins-g commented May 3, 2018

@xster / @briankung shlock's behavior in brew is a little surprising to me. It's not like much special is going on here:

https://opensource.apple.com/source/shell_cmds/shell_cmds-118/shlock/shlock.c.auto.html

openloop:
	if ((fd = open(tempname, O_RDWR|O_CREAT|O_EXCL, 0644)) < 0) {

That should always be valid to do on a POSIX filesystem -- well, unless you've set chflags on the directory to prevent it from being written:

jcollins-macbookpro:shlock jcollins$ sudo chflags uchg .
jcollins-macbookpro:shlock jcollins$ ls -la
total 0
drwxr-xr-x   2 jcollins  wheel    64 May  3 08:50 .
drwxrwxrwt  69 root      wheel  2208 May  3 08:52 ..
jcollins-macbookpro:shlock jcollins$ echo hi > foo
-bash: foo: Operation not permitted
jcollins-macbookpro:shlock jcollins$ ls -la
total 0
drwxr-xr-x   2 jcollins  wheel    64 May  3 08:50 .
drwxrwxrwt  69 root      wheel  2208 May  3 08:52 ..
jcollins-macbookpro:shlock jcollins$ shlock -f foo -p $$
shlock: open(shlock14741): Operation not permitted
jcollins-macbookpro:shlock jcollins$

If something in homebrew or the way you're setting it up does that, fixing shlock isn't going to cut it because you'll then fail when other write operations are inevitably attempted in the cache.

If adding this line:

chflags -R nouchg "$FLUTTER_ROOT/bin/cache"

here fixes it, you've got a chflags problem and probably should deal with that somehow. Somehow, bin/cache may already exist at the point you first run flutter (maybe you're starting with a binary build of flutter? Those have cache-premade and so they'll pass the mkdir fine, but you won't be able to write to the cache).

[edited to fix chflags]

@briankung

This comment has been minimized.

briankung commented May 3, 2018

@jcollins-g Ah, my unix-fu isn't up to snuff - I didn't know about chflags, but it makes sense. I suppose it's academic at this point, but I'll look into it. Thanks!

@briankung

This comment has been minimized.

briankung commented May 4, 2018

I added the line as follows:

diff --git a/bin/flutter b/bin/flutter
index 079c3f94d..1b7e06f43 100755
--- a/bin/flutter
+++ b/bin/flutter
@@ -39,6 +39,7 @@ function _rmlock () {
 
 function upgrade_flutter () {
   mkdir -p "$FLUTTER_ROOT/bin/cache"
+  chflags -R nouchg "$FLUTTER_ROOT/bin/cache"
 
   # This function is executed with a redirect that pipes the source of
   # this script into file descriptor 3.

And the test flutter --version still fails with the shlock error.

I missed something you said earlier:

maybe you're starting with a binary build of flutter? Those have cache-premade and so they'll pass the mkdir fine, but you won't be able to write to the cache

I think I am using a binary build of flutter. It's the zip from https://flutter.io/setup-macos/#get-sdk.

Since I was curious, I deleted the cache folder to see if the flutter script could create the folder itself. It could not: mkdir: /usr/local/Cellar/flutter/0.3.1-beta/bin/cache: Operation not permitted and inserting an echo $(whoami) into flutter shows that the script is running as my personal user (sanity check).

I'm not too familiar with Unix permissions, but it looks like I do need sudo to edit any files in /usr/local/Cellar/flutter/, and reviewing the permissions shows that all the files are in the admin group:

$ pwd && echo && ls -al bin
/usr/local/Cellar/flutter/0.3.1-beta

total 32
drwxr-xr-x   5 briankung  admin   160 May  4 10:01 .
drwxr-xr-x  26 briankung  admin   832 May  4 09:53 ..
-r-xr-xr-x   1 briankung  admin  6892 May  4 10:01 flutter
-r--r--r--   1 briankung  admin  6338 May  4 09:52 flutter.bat
drwxr-xr-x   8 briankung  admin   256 May  4 09:52 internal

Copying the directory into my home directory assigns it the staff group, and then running ~/flutter/bin/flutter --version with the cache directory deleted starts downloading the Dart language. However, changing the group on Homebrew's flutter folder, /usr/local/Cellar/flutter/, does not allow the brew keg to create the folder, and when I create the folder myself, I end up right back at the shlock: open(/usr/local/Cellar/flutter/0.3.1-beta/bin/cache/shlock9849): Operation not permitted error.

@jcollins-g

This comment has been minimized.

Contributor

jcollins-g commented May 4, 2018

Thanks for the detailed response @briankung. I could dive further into this with you, but it sounds like the fundamental bug is that we can't guarantee writeability into bin/cache under the flutter installation by regular users and Flutter assumes this.

Most Unix packages separate the cached bits they manage from the binary installation. Flutter's packaging method is a bit unorthodox in that the cached bits include most of the binary installation. Homebrew, apt-get, virtually all package managers and operating systems will have some combination of permissions, chroot during build, chattr/chflags, etc to try to prevent installed packages from being modified by regular users.

I think the right thing to do here might be to have a flag set in a file that locks the flutter cache as read-only, and make flutter's built-in update tools respect it. That way we can distinguish between user-configuration error and the user using an installed package, and display appropriate errors. Or, more simply, we could just check for cache writeability and skip it if we can't write to the cache.

@jcollins-g

This comment has been minimized.

Contributor

jcollins-g commented May 4, 2018

@xster's original suggestion might be enough if the rest of the tools already support this. which sounds like it might be the case.

@jcollins-g

This comment has been minimized.

Contributor

jcollins-g commented May 4, 2018

I've done a little more digging. It looks like we mostly do not need to write to FLUTTER_ROOT and its subdirectories.

However, there are exceptions that look likely to result in crashes:

The shlock we're running into here is definitely one.
The locking in flutter upgrade is another.
The locking for golden files is another.

Additionally, the preference for a flutter root pub cache looks suspicious to me -- unless pub is coded to handle this case that's probably not going to work for cases where a developer wants to use a different package, either.

Given that I found this many little issues right away, I think to fix this for good we probably need to add running integration tests using a read-only FLUTTER_ROOT and track down any little one-offs like this. That'll also keep this from cropping up again as it is really easy as a Flutter developer to accidentally introduce something like this.

@xster

This comment has been minimized.

Contributor

xster commented May 4, 2018

+1 on testing with read-only root

@HofmannZ

This comment has been minimized.

HofmannZ commented Jul 16, 2018

Any updates on this?

@Hixie Hixie changed the title from Let flutter be installable via homebrew/apt-get to Let flutter be installable via homebrew Aug 14, 2018

@Hixie

This comment has been minimized.

Contributor

Hixie commented Aug 14, 2018

This is not currently something we're planning to do, but if someone wanted to work on this we would support this work. I presume it would either involve integrating with our continuous integration scripts to publish to homebrew in some manner, or creating a bootstrap script that knows how to integrate with flutter, and maintaining that (and especially, writing tests to make sure that we know when it breaks, so we can fix it). If you are interested in working on this, please don't hesitate to reach out to me.

I've changed this bug to be specifically about homebrew, if you are interested in the same thing for apt-get please file a separate bug (though the comment above applies equally to that).

@Hixie Hixie modified the milestones: Goals, Stretch Goals Aug 14, 2018

@eclewlow

This comment has been minimized.

eclewlow commented Sep 16, 2018

I made a tap repo fo Mac OS: (https://github.com/eclewlow/homebrew-formulas)

All it does is install flutter, though.
brew install eclewlow/formulas/flutter

You still have to download IOS & Android Dev Tools yourself.

It didn't pass the brew audit though, so I can't add it to homebrew.
Here's why:

  * Stable version URLs should not contain beta
  * Non-executables were installed to "/usr/local/opt/flutter/bin"
    The offending files are:
      /usr/local/opt/flutter/bin/cache
      /usr/local/opt/flutter/bin/internal
  * flutter has broken dynamic library links:
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment