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

Chocolatey-Preinstaller-Checks.extension - REQUEST: Do not use aliases #29

Closed
ferventcoder opened this issue May 25, 2018 · 17 comments
Closed
Assignees

Comments

@ferventcoder
Copy link

ferventcoder commented May 25, 2018

Hi @bcurran3,
The use of aliases for extensions is not allowed if they are replacing original methods. Please find a different method for handling this. We want to ensure you don't break licensed editions and adding overriding aliases does this.

Set-Alias Install-ChocolateyInstallPackage Start-PreinstallChecks -Force -Scope Global
Set-Alias Uninstall-ChocolateyPackage Start-PreuninstallChecks -Force -Scope Global

@ferventcoder
Copy link
Author

Another way of putting it - if we see this, we won't approve those extensions.

@ferventcoder
Copy link
Author

Alternatively you can host on your own MyGet feed and do what you want for extensions there - just not in the community repository. Some of these have issues and we see a trend where we have customers requesting support or attempting to file issues because of something your extensions are doing that is breaking our functionality and we need to nip that in the bud as soon as possible.

@gep13
Copy link

gep13 commented May 25, 2018

@bcurran3 as an example of the sorts of problems that we are seeing...

With your extension installed, on my machine, which has a license installed, and the background service activated, I am seeing this...

image

@bcurran3
Copy link
Owner

bcurran3 commented May 26, 2018

(Yeah, you're setting me off again, but I use logic, reason, research and references...)

@ferventcoder

Take the time to look at all the scripts and don't prejudge based on the first (and probably only) one you took the time to view. The aliases are simply an intercept to inject and run new functions before Install-ChocolateyInstallPackage and Uninstall-ChocolateyInstallPackage. They are then unaliased and continue on to run Chocolatey functions 100% completely as normal, untouched.

Remove-Item alias:\Install-ChocolateyInstallPackage
Install-ChocolateyInstallPackage @args
Remove-Item alias:\Uninstall-ChocolateyPackage
Uninstall-ChocolateyPackage @args

There is 100% no way to break functionality; my extension adds the functionality of heading off and warning about possible problems before they occur. I think it's well thought out and rather genius. :) I'm the first to make an extension/helper that doesn't rely on a package maintainer to explicitly define and use a helper to add functionality. It's a true "add-on."

"Some of these have issues and we see a trend where we have customers requesting support or attempting to file issues because of something your extensions are doing that is breaking our functionality and we need to nip that in the bud as soon as possible.

Absolutely not true. There is no trend and there is nothing I've ever created that breaks any functionality. None of my original creations have ever had a single issue. If there were any truth to what you accuse, you'd find bug reports in my https://github.com/bcurran3/ChocolateyPackages/issues. Guess what? NOT A SINGLE BUG REPORT OPEN OR CLOSED. I call complete bullshit on you here. The scope of your comment is narrow, out of context, unfair, and not researched. You get support requests on a myriad of general packages, constantly and consistently. What you do have is one person in Gitter who had a problem with upgrading AutoHotKey. The root cause for the problem is a bug in Chocolatey that has been around for a long time. When an upgrade gets aborted (such as Ctrl-C) Chocolatey doesn't return the original package files from lib-bad to lib and causes Chocolatey to loose management of that package. There is a two year running issue about this documented here: chocolatey/choco#780, here: chocolatey/choco#619, here: chocolatey/choco#1548 as well as here: https://groups.google.com/forum/#!searchin/chocolatey/reboot|sort:date/chocolatey/zsMuLUSzSwE/HMyAnVg9DAAJ An abort task in some of my scripts may be the instigator for an upgrade to forcefully abort after an unreasonable (and completely user opted in or specifically configured) amount of time, but it's just the symptom of the underlying root cause that Chocolatey doesn't handle an external abort gracefully. Additionally you had one person mention one of my helpers at chocolatey/choco#544 where it helped him debug and confirm his needed reboot situation. It didn't hinder him or affect any kind of Chocolatey functionality. Conversely, it did what a helper should do, it helped him. :) Neither of these MENTIONS of my extension and script justify "...we see a trend where we have customers requesting support or attempting to file issues because of something your extensions are doing that is breaking our functionality..." because everything in that statement is false.

Take a full look, not a knee jerk reaction please. I'm sure you're tired and overworked, but give me the respect of delving fully into and researching "the problem."

@gep13

...an example of the sorts of problems that we are seeing...

No, not "we." You. The extension has been downloaded at most 4 times outside of the validator, myself, and you. What you are seeing is a feature which is an ingenious :) temporary kludge until Chocolatey fully supports multi-instances of itself without causing config or package destruction. See chocolatey/choco#1579 and all it's previous references. . You are subjectively calling it a problem based on YOUR atypical environment. For what you are seeing, you are in the wrong issue. Please add your thoughts about what you are seeing to #27 for the features in v0.0.2.

You're a Chocolatey developer and running a licensed version, you are far from the typical FOSS user. Your scenario is atypical. The beauty of this extension is that it is built for Chocolatey FOSS end users, has no use as a dependency, will never be referenced as a dependency, and will never be involuntarily or unknowingly installed. The package is for the community repository which is constantly referred to as "not recommended for business" by Rob. FOSS non-licensed users will not have the background service running. All packages in the community repository are opt-in. No one forces them to install or use them, it's personal choice. Forced packages/non-voluntarily installed packages come in to play in the business world, refer to above ad nauseam, or better yet https://chocolatey.org/docs/community-packages-disclaimer, https://chocolatey.org/about, or even https://chocolatey.org/security which I will quote below.

Chocolatey Community Package Repository Use of the community package repository is optional. Community package repository is the same thing as Chocolatey.org packages, and represents less than 5% of the existing packages in existence (nearly all are internal). Most organizations using Chocolatey do NOT use the community repository, and Chocolatey Software DOES NOT RECOMMEND using the community repository either for organizational deployments for a variety of reasons.

Thank you 1 for actually installing/testing it. Thank you 2 for unintentionally letting me know to put a disclaimer in for the highly improbable scenario where someone might attempt to use the extension with the self service agent running. :)

Not that I'm asking you to change the name for compatibility for my insignificant little extension, but doesn't it make more sense to name the Chocolatey background agent something like "chocoagent.exe" instead of "choco.exe?"

@ferventcoder and @gep13

So please help me better understand how my opt-in extension, being 1/5820th of the 5% of existing packages and introduces new functionality, not breaks, can affect C4B customers who are explicitly warned in many locations not to use the community repository which in turn is considered "optional?" Not to mention that these C4B customers are really IT professionals who would, if they considered using my extension, test the package and then make an experienced post-test-environment decision on using it or not before deploying to their organization. Where's the "problem?" I don't see it. I see an OPTION.

I'll await further comments. I believe if you read what I said and look closely at the extension I wrote, I'm confident you'll have a better understanding and no objections.

Yes, I'm insulted that you're responses, IMHO were offbase and apparently not researched. I'm a big supporter of Chocolatey and a fan of you two. I am surprised that you came across so heavy-handed (but would expect it 100% if there were an actual problem). I've created 17 of ~29 3rd party Chocolatey add-ons, including this extension, that further the betterment of the Chocolatey FOSS community. (EDIT: That ~29 # is actually lower as the majority of what I counted were packages released by Chocolatey Team members under their own accounts. The number of "3rd party" contributions is closer to 5. So I've contributed 17 of 22.) Obviously I'm benefiting the Chocolatey FOSS community as people seem to like my "stuff" since they install it and continue to upgrade it. Some of them even say "Thank you" to me. You talk of "support" issues when I ACTIVELY man the Chocolatey support forum and commonly answer questions within my scope of knowledge helping other users and end up actually reducing your support time/role. Does it do anything to benefit me? No. Does it benefit you? Yes. Compare the numbers, I think you'll find out I have more plus points under the provided support category than the fictitious generated support category. Do I get thanked or even acknowledged? No. Hell, I've got more open questions there than anyone else!

Do you realize I have hundreds of read.md files that say "If applicable, please always consider donating to the developer or purchasing the software first - this includes Chocolatey licensed editions." Do you realize that I have 3rd party Chocolatey add-ons that are for licensed users only and I actively link and promote the licensed editions everywhere I can in the package files? I even have a "Write-Host Chocolatey license file found. Thanks for supporting Chocolatey! -foreground green" in my choco-optimize-at package! Do I get a little cross-promotional public acknowledgement, thanks, reference, support, or "Chocolatey 3rd Party Developer of the Year" award (Yeah, impossible to beat out @mwrock - 'specially since his work's going to be helping to drive revenue.) to point me and my work out in hopes of getting some donations in my pocket? No. So that leads to...

(from gitter) EDIT: "...but we can either ask @bcurran3 to remove all of those and host elsewhere which I'm not necessarily a fan of I guess is a sign of some sort of support. Yes Rob, the "fair" reply to the suggestion of moving my script packages to another repository outside of the Chocolatey community repository was very disheartening to me. It came across the opposite of what I would have hoped. As you can see I worked it out with @wget and at least HE doesn't feel that way anymore. (Where's the "feeling very underappreciated" emoji?)

FYI: No, I'm not a touchy feely guy. My statements above are honest and true, but I'm not sitting here crying or rampaging. I'm trying to bring reason to an otherwise unreasonable conversation... and pondering why I put so much volunteered time and effort into someone else's business with very little benefit to myself.

Tell me you were drunk when you posted and you won't have to say "You're right Bill." :)

@bcurran3
Copy link
Owner

bcurran3 commented May 26, 2018

OFF TOPIC: Is there an issues page for the Chocolatey agent as I have some requests. :)
On my TDL is the idea of a Windows service that is an RSS reader to compare the community feed to installed packages and do instant upgrades. :)

And can you provide any guidance on comment number 7 here: #27 ? :)

@bcurran3 bcurran3 changed the title Do not use aliases Chocolatey-Preinstaller-Checks.extension - Do not use aliases May 26, 2018
@bcurran3 bcurran3 changed the title Chocolatey-Preinstaller-Checks.extension - Do not use aliases Chocolatey-Preinstaller-Checks.extension - REQUEST: Do not use aliases May 26, 2018
@ferventcoder
Copy link
Author

ferventcoder commented May 30, 2018

The aliases are simply an intercept to inject and run new functions before Install-ChocolateyInstallPackage and Uninstall-ChocolateyInstallPackage. They are then unaliased and continue on to run Chocolatey functions 100% completely as normal, untouched.

In licensed editions, the alias is added for Install-ChocolateyInstallPackageCmdLet with the alias Install-ChocolateyPackage. So the licensed code which has CDN support, virus checking, and overriding the installation directory would all cease to function with your code (I just reviewed it closer).

Note Licensed Editions - Pro Users, who do use the community repository.

There is 100% no way to break functionality; my extension adds the functionality of heading off and warning about possible problems before they occur. I think it's well thought out and rather genius. :) I'm the first to make an extension/helper that doesn't rely on a package maintainer to explicitly define and use a helper to add functionality. It's a true "add-on."

I'd agree it's well thought out and pretty cool - the problem is that it does remove all licensed functionality if it overrides an alias we've overridden, and it does it in a totally non-obvious way. There in lies the issue. We start to get issues filed that someone didn't get a CDN download they expected, or worse, the runtime virus scan check and now they are worried about malware. They want to blame something on our side. After a lot of back and forth with support tickets we realize it is because they have one of your extensions (or another extension installed that also takes this precedent of overriding aliases). We close the ticket as invalid but no one is happy. We don't like invalid support tickets, its a waste of time on my techs and we don't get that time back. You are currently overriding the Install-ChocolateyPackage and we are overriding Install-ChocolateyInstallPackage at this point (which means the two co-exist at this point). However as we move into more of those functions going over to licensed code, more of this goes away. We either suggest finding a different way now, or we remove your extensions later when they do conflict. I'm not sure which one is going to make you more angry, but at the end of the day we need to ensure compatibility for folks that are paying for that functionality.

The package is for the community repository which is constantly referred to as "not recommended for business" by Rob. FOSS non-licensed users will not have the background service running. All packages in the community repository are opt-in. No one forces them to install or use them, it's personal choice. Forced packages/non-voluntarily installed packages come in to play in the business world, refer to above ad nauseam, or better yet https://chocolatey.org/docs/community-packages-disclaimer, https://chocolatey.org/about, or even https://chocolatey.org/security which I will quote below.

See above, we have a large group of folks using the community repository - Professional Users. All of whom lose functionality.

Most organizations using Chocolatey do NOT use the community repository, and Chocolatey Software DOES NOT RECOMMEND using the community repository either for organizational deployments for a variety of reasons.

Taking this strawman for a moment, organizations do internalize packages from the community repository. What if one of those packages takes a dependency on your extension or another that would also use the precedent of alias overrides?

You are in the here and now, I'm ten steps ahead at foreseeing where this would go and what issues it causes. That's what I'm working to prevent.

@ferventcoder
Copy link
Author

ferventcoder commented May 30, 2018

Not that I'm asking you to change the name for compatibility for my insignificant little extension, but doesn't it make more sense to name the Chocolatey background agent something like "chocoagent.exe" instead of "choco.exe?"

The background service is named chocolatey-agent.exe - but it does run choco.exe as part of what it does.

Chocolatey Agent Service:
image

Running choco.exe runs a shim and an actual choco.exe (both are named choco.exe - likely causing issues):
image

This was also noted at at #27 (comment)

@ferventcoder
Copy link
Author

So please help me better understand how my opt-in extension, being 1/5820th of the 5% of existing packages and introduces new functionality, not breaks, can affect C4B customers who are explicitly warned in many locations not to use the community repository which in turn is considered "optional?" Not to mention that these C4B customers are really IT professionals who would, if they considered using my extension, test the package and then make an experienced post-test-environment decision on using it or not before deploying to their organization. Where's the "problem?" I don't see it. I see an OPTION.

Not just C4B, every pro user that does use the community repository.

@ferventcoder
Copy link
Author

ferventcoder commented May 30, 2018

So currently we override Install-ChocolateyInstallPackage (and a couple of other methods), but our plan is to override more in the licensed editions. As we do that, it's going to break for non-obvious reasons between the two.

@ferventcoder
Copy link
Author

ferventcoder commented May 30, 2018

So bringing this back to my original request - overriding aliases for built-in methods can't be allowed on the community repository due to support and compatibility concerns.

You currently override Install-ChocolateyPackage and licensed extensions override Install-ChocolateyInstallPackage (and a handful of others). This means this extension co-exists in some ways with licensed code at this point in time. However as we move into more of those functions going over to licensed code, more of this goes away. We either suggest finding a different way now, or we remove your extensions later when they do conflict. I'm not sure which one is going to make you more upset, but at the end of the day we need to ensure compatibility for folks that are paying for that functionality.

Yes, I'm insulted that you're responses, IMHO were offbase and apparently not researched. I'm a big supporter of Chocolatey and a fan of you two. I am surprised that you came across so heavy-handed (but would expect it 100% if there were an actual problem).

I believe requesting that you find a different way of handling this now would be a better option with less consternation. Perhaps I was incorrect in that assessment.

@ferventcoder
Copy link
Author

You talk of "support" issues when I ACTIVELY man the Chocolatey support forum and commonly answer questions within my scope of knowledge helping other users and end up actually reducing your support time/role. Does it do anything to benefit me? No. Does it benefit you? Yes. Compare the numbers, I think you'll find out I have more plus points under the provided support category than the fictitious generated support category. Do I get thanked or even acknowledged? No. Hell, I've got more open questions there than anyone else!

Support issues on private channels is more so where I'm talking. So we have categories of support, including guaranteed response time direct support for licensed customers - it's all defined from FOSS all the way up to C4B at https://chocolatey.org/support. Unless you work for Chocolatey Software, you won't see those private channels. 😄

@bcurran3
Copy link
Owner

bcurran3 commented May 31, 2018

@ferventcoder

In licensed editions, the alias is added for Install-ChocolateyInstallPackageCmdLet with the alias Install-ChocolateyPackage. So the licensed code which has CDN support, virus checking, and overriding the installation directory would all cease to function with your code (I just reviewed it closer).
I'd agree it's well thought out and pretty cool - the problem is that it does remove all licensed functionality if it overrides an alias we've overridden, and it does it in a totally non-obvious way. There in lies the issue. We start to get issues filed that someone didn't get a CDN download they expected, or worse, the runtime virus scan check and now they are worried about malware. They want to blame something on our side. After a lot of back and forth with support tickets we realize it is because they have one of your extensions (or another extension installed that also takes this precedent of overriding aliases). We close the ticket as invalid but no one is happy. We don't like invalid support tickets, its a waste of time on my techs and we don't get that time back. You are currently overriding the Install-ChocolateyPackage and we are overriding Install-ChocolateyInstallPackage at this point (which means the two co-exist at this point). However as we move into more of those functions going over to licensed code, more of this goes away. We either suggest finding a different way now, or we remove your extensions later when they do conflict. I'm not sure which one is going to make you more angry, but at the end of the day we need to ensure compatibility for folks that are paying for that functionality.

This should address your concerns:

Remove-Item alias:\Install-ChocolateyInstallPackage
if ($env:ChocolateyLicenseValid -eq 'true') {
    Set-Alias Install-ChocolateyInstallPackage Install-ChocolateyInstallPackageCmdlet -Force -Scope Global
   }
Install-ChocolateyInstallPackage @args

Easy peasy.

`Taking this strawman for a moment, organizations do internalize packages from the community repository. What if one of those packages takes a dependency on your extension or another that would also use the precedent of alias overrides?

This has been covered previously above. Summarized: This is not a dependency and those who might internalize are IT professionals who would test this before enterprise wide deployment.

All negated now as my alias removal and restore now takes into account both unlicensed and licensed versions of Chocolatey. Your concern has been acted upon and is now handled.

You are in the here and now, I'm ten steps ahead at foreseeing where this would go and what issues it causes. That's what I'm working to prevent.

Of course you are the Chocolatey seer and enabler. I only know what is publically announced. This is a kludge/stop-gap solution until Chocolatey v1.0 release which has these issues on the roadmap to be addressed. At that point this package would be rendered moot, obsolete, and thus delisted based on current form.

@gep13

I've added the following into the checking for choco multiple instances:

if((get-process "choco-agent" -ea SilentlyContinue) -eq $Null){
    } else {
    Write-Host "  * choco-agent.exe found running, can't effectively check for multiple instances at this time. (Sorry!) " -foreground yellow
	break
  }

Easy peasy.

@ferventcoder @gep13 I believe the above addresses all your concerns.

I'd like to test the choco-agent and possible handle it in a more productive way (I'm guessing $AllowedChocos + 1 should do the trick.), but my Pro license expired at an unfortunate time, 5 days ago. :( I'm currently unable to test licensed functionality though the above changes should work fine in theory and do work fine in unlicensed testing. I'd appreciate it if you could test and let me know the results. :)

You can review everything fully at https://chocolatey.org/packages/chocolatey-preinstaller-checks.extension/0.0.1

Hopefully I'll soon have time to work on Chocolatey-Postinstaller-Checks.extension. 👍

@bcurran3 bcurran3 self-assigned this May 31, 2018
@bcurran3
Copy link
Owner

bcurran3 commented May 31, 2018

The background service is named chocolatey-agent.exe - but it does run choco.exe as part of what it does.

Thanks. Ooops, need to repush since I read "choco-agent" instead of "chocolatey-agent." If I could find out how many instances of choco.exe run due to the agent and a little more about what they are doing/not doing - I could handle the occurance in my script. I'm guessing whatever it's doing in the background is non-conflicting and non-destructive or it wouldn't be running; i.e. I probably just need to ignore one more instance of choco.exe when chocolatey-agent is found and checking for multiple instances.

Support issues on private channels is more so where I'm talking. So we have categories of support, including guaranteed response time direct support for licensed customers - it's all defined from FOSS all the way up to C4B at https://chocolatey.org/support. Unless you work for Chocolatey Software, you won't see those private channels. 😄

I'm aware of them, as a (former) licensed user I always use the public channels to help make problems and solutions more easily findable for future reference.

Chocolatey Software, Inc., something I strongly believe in and root for. I think it would be a good place to work. Put it in your budget and find a useful place for me as I'd love to work for Chocolatey Software, Inc. I'd be trying to win that employee of the month virtual parking spot. 😄

@bcurran3
Copy link
Owner

if((get-process "chocolatey-agent" -ea SilentlyContinue) -eq $Null){
    } else {
    Write-Host "  * chocolatey-agent.exe found running, can't effectively check for multiple instances at this time. (Sorry!) " -foreground yellow
	break
  }

Done.

@bcurran3
Copy link
Owner

This has been sitting 3 weeks. @ferventcoder @gep13 Please comment.

@bcurran3
Copy link
Owner

@ferventcoder and @gep13 - I summon you.

https://chocolatey.org/packages/chocolatey-preinstaller-checks.extension/0.0.1 has been updated yet again. There's more commenting for YOU to read and a whole new improved method of getting accurate choco.exe instance counts.

Let's address the white elephant in the moderation room and get this sucker passed.

@bcurran3
Copy link
Owner

...I need to buy this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants