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

Improve plist handling for hdiutil commands #5063

Merged
merged 1 commit into from
Jun 26, 2014

Conversation

rolandwalker
Copy link
Contributor

This has the incidental effect of emitting DMG licenses during
installation, which seems desirable as permanent functionality.

If not permanent, the warnings to STDERR should still be kept
temporarily to help get better bug reports on hdiutil.

A bug wrt DMG licenses must have been introduced in one of
#4892, #4887, #4889, #4900, #4975, #4978, or #4857. Presumably,

the cause is that STDERR was previously silenced when running
hdiutil. It would be cleaner (and more reliable) to redirect
STDERR and examine it separately, rather than clean up the
merged outputs.

closes #4819
closes #5060

- Raise `CaskError` instead of `Plist::ParseError` from module
- Improve error message when parse result is empty
- remove leading garbage text and emit it to stderr (seen in Homebrew#5060)
- remove trailing garbage text and emit it to stderr (seen in Homebrew#4819)

This has the incidental effect of emitting DMG licenses during
installation, which seems desirable as permanent functionality.

If not permanent, the warnings to STDERR should still be kept
temporarily to help get better bug reports on `hdiutil`.

A bug wrt DMG licenses must have been introduced in one of
Homebrew#4892, Homebrew#4887, Homebrew#4889, Homebrew#4900, Homebrew#4975, Homebrew#4978, or Homebrew#4857.  Presumably,
the cause is that STDERR was previously silenced when running
`hdiutil`.  It would be cleaner (and more reliable) to redirect
STDERR and examine it separately, rather than clean up the
merged outputs.

closes Homebrew#4819
closes Homebrew#5060
rolandwalker added a commit that referenced this pull request Jun 26, 2014
Improve plist handling for `hdiutil` commands
@rolandwalker rolandwalker merged commit 2396a0d into Homebrew:master Jun 26, 2014
@rolandwalker rolandwalker deleted the hdiutil_plist_handling branch June 26, 2014 00:02
@vitorgalvao vitorgalvao mentioned this pull request Jul 4, 2014
@vitorgalvao
Copy link
Member

This has the incidental effect of emitting DMG licenses during installation, which seems desirable as permanent functionality.

As @alebcay pointed out in #5191, the message is “a bit startling, probably especially so to new users”. I agree. If even contributors were left to wonder what that was (I also encountered that recently, with Charles), I imagine many users will as well, specially since this does not happen with every cask. I also worry it will have the adverse effect of making users ignore important messages in other casks (i.e., the caveats): after seeing a bunch of text they don’t care about, a few times, they’ll be more prone to ignore future messages.

I’d also like to revive the discussion on dmg agreements. My opinion has not changed: I still believe we should not install apps without the user agreeing, unless a specific option is set.

@rolandwalker
Copy link
Contributor Author

OK, I will remove this accidental "feature".

Though we aren't entirely on the same page wrt DMG agreements, I'm open to your argument.

However, one thing we cannot do is stop and interactively query as hdiutil likes to do. We should revisit DMG agreements once we have a persistent config file.

@vitorgalvao
Copy link
Member

I would very much like your opinion on the issue. Particularly now, that we’ve already decided the fate of one cask based on its legality, I find that accepting legal agreements on behalf of the user by default is not something we should be doing.

@Homebrew Homebrew locked and limited conversation to collaborators May 8, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error: undefined method `ParseError' for Plist:Module install google-chrome failed
2 participants