brew install can fail when running under absurdly large gid

[mcornick]

Here's a weird one for you - heroku-toolbelt 3.42.20 install is breaking immediately after untarring the source: https://gist.github.com/30bf2271c66d6fe1f8a3

This doesn't appear to be a problem with the source tar, since if I untar heroku-toolbelt-3.42.20.tgz outside of Homebrew, I get no errors.

This is on Yosemite 10.10.5, with Homebrew/homebrew@3c9da0212be124c6aa7d2e14ed1c13b14767f01e. brew doctor reports no issues. brew gist-logs heroku-toolbelt returns no logs.

[mcornick]

I've tried this on an El Capitan 10.11 system, and heroku-toolbelt installs without issues.

[DomT4]

Strange. Presume the tar in your $PATH is the system one?

[mcornick]

Yep, which tar returns /usr/bin/tar. I do have the gnu-tar brew installed, which provides /usr/local/bin/gtar, but removing that makes no difference to the heroku-toolbelt install.

[DomT4]

Can you try tar zxvf /Library/Caches/Homebrew/heroku-toolbelt-3.42.20.tgz? Might output a bit more information on where the hiccup is.

[mcornick]

Sure: https://gist.github.com/28224a1f64b451b34006

[DomT4]

Hmm. That looks like it untarred okay. Did it still throw the failure?

[mcornick]

No, that didn't produce any errors. Seems like there must be something edge-case-y about that tar?

[mcornick]

OK, I think I've finally tracked this down.

First off, this doesn't have anything to do with heroku-toolbelt specifically. I'm getting exactly the same error (integer 4294967295 too big to convert to int) trying to install brew-cask (which I know is third-party, but still, an entirely different formula.) And that formula installs from a git clone, not a tar. See https://gist.github.com/markcornick/612fe7297827d0cc9a09

/usr/local/Library/Homebrew/extend/pathname.rb:118:inatomic_write'` has to do with chown'ing/chgrp'ing a Tempfile. It turns out 4294967295 is the gid my shell is running under:

uid=500(mcornick) gid=4294967295(nogroup) groups=12(everyone),61(localaccounts),79(_appserverusr),80(admin),81(_appserveradm),98(_lpadmin),33(_appstore),100(_lpoperator),204(_developer),395(com.apple.access_ftp),398(com.apple.access_screensharing),399(com.apple.access_ssh)

I have no idea how that happened, and I'll get it fixed on my end, but brew could probably be a little more resilient in this (admittedly very edge) case. Not sure if/how you want to handle that.

[DomT4]

@xu-cheng Anything practical we can do about the above?

[xu-cheng]

Looks to me a bug in Ruby interpreter rather a Homebrew. @markcornick if you can reproduce this bug outside Homebrew, you may want to report it to Ruby.

[apjanke]

I have no idea how that happened,

4,294,967,295 is the maximum value of an unsigned 32-bit integer. And on OS X (10.9 at least), gid_t is an unsigned int. So 4,294,967,295 is (gid_t) -1, and for chown, (gid_t) -1 is reserved as a sentinel value to indicate "omitted argument" or "do not change".

If owner or group is specified as ( uid_t)-1 or ( gid_t)-1, respectively, the corresponding ID of the file shall not be changed. If both owner and group are -1, the times need not be updated.

I don't see a mention of (gid_t) -1 in setgid and its ilk, but I suspect it works the same way. Maybe your shell is running with no primary group rights, or some function failed to see (gid_t) -1 as a sentinel value, and used it as a normal gid value, maybe when setting your process's effective UIDs and GIDs?

Looks to me a bug in Ruby interpreter rather a Homebrew.

This may be a bug in File in Ruby's standard library that is already fixed in newer (2.3+) versions of Ruby.

From file.c in the Ruby 2.0.0 source tree:

/*
 *  call-seq:
 *     file.chown(owner_int, group_int )   -> 0
 *
 *  Changes the owner and group of <i>file</i> to the given numeric
 *  owner and group id's. Only a process with superuser privileges may
 *  change the owner of a file. The current owner of a file may change
 *  the file's group to any group to which the owner belongs. A
 *  <code>nil</code> or -1 owner or group id is ignored. Follows
 *  symbolic links. See also <code>File#lchown</code>.
 *
 *     File.new("testfile").chown(502, 1000)
 *
 */

static VALUE
rb_file_chown(VALUE obj, VALUE owner, VALUE group)
{
    rb_io_t *fptr;
    int o, g;
#ifndef HAVE_FCHOWN
    VALUE path;
#endif

    rb_secure(2);
    o = NIL_P(owner) ? -1 : NUM2INT(owner);
    g = NIL_P(group) ? -1 : NUM2INT(group);
    GetOpenFile(obj, fptr);
#ifndef HAVE_FCHOWN
    if (NIL_P(fptr->pathv)) return Qnil;
    path = rb_str_encode_ospath(fptr->pathv);
    if (chown(RSTRING_PTR(path), o, g) == -1)
    rb_sys_fail_path(fptr->pathv);
#else
    if (fchown(fptr->fd, o, g) == -1)
    rb_sys_fail_path(fptr->pathv);
#endif

    return INT2FIX(0);
}

Note this part:

    int o, g;
    [...]
    o = NIL_P(owner) ? -1 : NUM2INT(owner);
    g = NIL_P(group) ? -1 : NUM2INT(group);

NUM2INT returns a signed int, not an unsigned int. So it doesn't line up with the actual range of gid_t, and in particular, your 4,294,967,295 falls outside it. There's a separate NUM2GIDT and rb_gid_t that is used in the multi-file rb_file_s_chown, and should probably be used here, too.

Looks like this was fixed for Ruby 2.3, with this function switching to use rb_gid_t.

This bug report search suggests that this fix was backported to Ruby 2.0.0. However, Ruby 2.0.0-p647 has not picked it up, so it'll be in a later backport release. And Apple doesn't seem to update the Ruby on released OS X systems anyway, so this probably won't help us.

As a workaround for Homebrew, I think we could switch to using the File.chown(uid, gid, file, ...) function form instead of the file.chown(uid, gid) object-method form. It already uses the right rb_gid_t types in older Ruby 2.0.0 builds, maybe including the ones shipped with OS X.

[MikeMcQuaid]

As a workaround for Homebrew, I think we could switch to using the File.chown(uid, gid, file, ...) function form instead of the file.chown(uid, gid) object-method form. It already uses the right rb_gid_t types in older Ruby 2.0.0 builds, maybe including the ones shipped with OS X.

Amazing spelunking here @apjanke. A PR would be great :+1:

[DomT4]

I should close issues more often, managed to get three extra maintainers involved :trollface:

[apjanke]

Cool. Would it be appropriate to use the extend mechanism to just override File.chown within Homebrew so it picks it up everywhere? Or just change the individual chown calls within core?

[MikeMcQuaid]

Cool. Would it be appropriate to use the extend mechanism to just override File.chown within Homebrew so it picks it up everywhere? Or just change the individual chown calls within core?

I'd be OK with either.