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 Adobe Creative Cloud uninstall #68039

Merged
merged 1 commit into from
Sep 17, 2019

Conversation

danielbayley
Copy link
Contributor

Following on from #67745, which was merged prematurely… this PR cleans up all of the leftover crap from Adobe CC.

After making all changes to the cask:

  • brew cask audit --download {{cask_file}} is error-free.
  • brew cask style --fix {{cask_file}} reports no offenses.
  • The commit message includes the cask’s name and version.
  • The submission is for a stable version or documented exception.

Additionally, if adding a new cask:

@danielbayley danielbayley force-pushed the adobe-creative-cloud branch 4 times, most recently from ad86287 to 2f5a407 Compare August 23, 2019 15:35
@danielbayley
Copy link
Contributor Author

I don't understand why this is failing when I have must_succeed: false? It doesn't help that I can't seem to run brew cask ci offline…

@reitermarkus
Copy link
Member

You can run it with env CI=1 brew cask ci, if you really need to. brew cask uninstall should fail with the same error though.

Casks/adobe-creative-cloud.rb Outdated Show resolved Hide resolved
Casks/adobe-creative-cloud.rb Outdated Show resolved Hide resolved
Casks/adobe-creative-cloud.rb Outdated Show resolved Hide resolved
@danielbayley danielbayley force-pushed the adobe-creative-cloud branch 5 times, most recently from 38d79a6 to 9574ca4 Compare August 25, 2019 20:16
Casks/adobe-creative-cloud.rb Outdated Show resolved Hide resolved
Casks/adobe-creative-cloud.rb Outdated Show resolved Hide resolved
@danielbayley
Copy link
Contributor Author

danielbayley commented Aug 26, 2019

metric crapton of referenced paths

That’s Adobe for you 🙄

What files does this remove?

If you're talking about this line: '~/Library/Preferences/Adobe/.[A-Z0-9]???????????’,, some random dotfile that this app creates… if it stays in place then rmdir: will obviously fail. I’m not making these up 😅 Adobe scatter this shit all over the place, and even their own official uninstaller app doesn’t get rid of half of it properly…

or run the whole command in system_command with system tools.

@vitorgalvao What do you mean? I don’t see how this is possible given that you can’t use | pipes in system_command; the second call to bin/launchctl works from the stdout of the first…

remove is a deprecated launchctl command.

So is list incidentally… but it’s still being used in the core code, here for one example. Looking at the man launchctl page… would kill, or bootout be the replacement for remove?

@vitorgalvao
Copy link
Member

That’s Adobe for you 🙄

I hear you!

What do you mean? I don’t see how this is possible given that you can’t use | pipes in system_command

Yes, you’re right.

Looking at the man launchctl page… would kill be the replacement?

bootout should do it. But now that I think of it, why aren’t you using uninstall launchctl for that?

Copy link
Contributor Author

@danielbayley danielbayley left a comment

Choose a reason for hiding this comment

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

why aren’t you using uninstall launchctl

@vitorgalvao Because for the processes I haven’t already included in that stanza, it doesn’t work…

> brew cask uninstall adobe-creative-cloud
#
> launchctl print system | grep -i adobe

"com.adobe.GC.Scheduler-1.0" => false
"com.adobe.fpsaud" => true

and

> launchctl print gui/$UID | grep -i adobe

		       0      - 	com.adobe.CCXProcess.61720
		       0      - 	com.adobe.CCXProcess.61380
		       0      - 	com.adobe.CCXProcess.61636
		   14864      - 	com.adobe.CCXProcess.61716
		       0      - 	com.adobe.CCXProcess.61532
				"com.adobe.accmac.explinder.ACCFinderInnerExtensionHost2.14846" = {
				"com.adobe.accmac.explinder.ACCFinderInnerExtensionHost2.14845" = {
		 0xcbe3b    U   A   com.adobe.accmac.explinder.ACCFinderInnerExtensionHost2.14845
		 0xed07b    U   A   com.adobe.accmac.explinder.ACCFinderInnerExtensionHost2.14846
	"com.adobe.AdobeCreativeCloud" => true
	"com.adobe.GC.Scheduler-1.0" => false

Apparently they generate a random 5 digit suffix for each process… just to be even more of a pain in the ass 🙄 Fuck Adobe backwards.

Unfortunately glob patterns aren’t available with uninstall launchctl:. Also signal: with any of the SIGs doesn’t work either… including even KILL.

Casks/adobe-creative-cloud.rb Outdated Show resolved Hide resolved
@danielbayley danielbayley force-pushed the adobe-creative-cloud branch 6 times, most recently from 0d185f9 to 645ccc2 Compare August 27, 2019 18:29
@danielbayley
Copy link
Contributor Author

@vitorgalvao I think this is as good as we’re going to get now with this…

@reitermarkus
Copy link
Member

Apparently they generate a random 5 digit suffix for each process

They aren't, this is done by launchctl I think, and this should already be handled by:

https://github.com/Homebrew/brew/blob/d5cc46f931c4db200b0ca9e119411dd7a6721da8/Library/Homebrew/cask/artifact/abstract_uninstall.rb#L123

@danielbayley
Copy link
Contributor Author

danielbayley commented Aug 30, 2019

this is done by launchctl I think, and this should already be handled

@reitermarkus Interesting. Looks like it should, but it isn't… even if I put both

'com.adobe.CCXProcess',
'com.adobe.ccxprocess',

in the launchctl: [array], without the uninstall_postflight code, com.adobe.CCXProcess.*es are still left running after brew cask uninstall, according to launchctl list | grep -i adobe. Could this be a bug with that part of the core code?

This current iteration is the only way I managed to find that both passes the CI and cosistently removes every process/piece of leftover Adobe crap I could find relating directly to CC.

@reitermarkus
Copy link
Member

Could this be a bug with that part of the core code?

The core code is not using launchctl bootout.

@danielbayley
Copy link
Contributor Author

The core code is not using launchctl bootout.

@reitermarkus Right, but as mentioned by @vitorgalvao remove is a deprecated launchctl command… so what to do?

@reitermarkus
Copy link
Member

remove is a deprecated launchctl

Since when?

@danielbayley
Copy link
Contributor Author

Since when?

It's listed as a legacy subcommand on the man page… but since other code seems to use list (another apparently deprecated subcommand) I don't see how this is a particular blocker to this formula. @vitorgalvao brought it up… seems like a separate issue from this PR.

@reitermarkus I’m not sure what the blocker/s is/are now?

@reitermarkus
Copy link
Member

So is bootstrap/bootout a direct replacement for load/unload?

I’m not sure what the blocker/s is/are now?

That depends on whether we should change the core code to use bootout instead of unload.

Following on from Homebrew#67745, which was merged prematurely… this PR cleans up all of the leftover crap from Adobe CC.
@danielbayley
Copy link
Contributor Author

So is bootstrap/bootout a direct replacement for load/unload?

I'm not sure on the details of this… you'll have to ask @vitorgalvao who raised it.

That depends on whether we should change the core code to use bootout instead of unload.

Surely that's a seperate issue to this PR… why now all of a sudden is this an issue? @vitorgalvao

@reitermarkus
Copy link
Member

why now all of a sudden is this an issue?

Because then we wouldn't need the workaround in uninstall_postflight in the first place.

@danielbayley
Copy link
Contributor Author

danielbayley commented Sep 14, 2019

Because then we wouldn't need the workaround in uninstall_postflight in the first place.

I wouldn't be so sure about that… cleaning up these Adobe processes feels like playing whack-a-mole at best! @reitermarkus Are you saying that you think because the underlying code behind the launchctl: stanza is [presumably] using unload, that if this was updated to use bootout then launchctl: would be sufficient to unload all of the given bundle IDs on it's own, without uninstall_postflight?

Have there been other casks where launchctl: (or signal:) on it's own has failed?

@reitermarkus
Copy link
Member

I wouldn't be so sure about that…

Well, I'm not.

if this was updated to use bootout then launchctl: would be sufficient

Probably, but that's the open question.

Have there been other casks

I don't remember any, probably only Adobe manages to pull that shit off.

@danielbayley
Copy link
Contributor Author

I don't remember any, probably only Adobe manages to pull that shit off.

😅 Probably 🙄

Probably, but that's the open question.

So this PR is basically now reliant on a new separate issue, which is to update all of the Homebrew internals to remove apparently deprecated launchctl commands?

rmdir: [
"#{appdir}/Adobe Creative Cloud",
'/Applications/Utilities/Adobe Installers',
'/Library/Application Support/Adobe{/CEP{/extensions,},}',
Copy link
Member

Choose a reason for hiding this comment

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

I still think some of these are a bit hard to parse, but fine, it’s Adobe, that you went through all this trouble is already great. Thank you. Let’s get this merged.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I still think some of these are a bit hard to parse

@vitorgalvao Yeah brace expansion can be tricky… this line is an example of why I opened #67806.

@vitorgalvao vitorgalvao merged commit cba0e9b into Homebrew:master Sep 17, 2019
@lock lock bot added the outdated label Jan 14, 2020
@lock lock bot locked as resolved and limited conversation to collaborators Jan 14, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants