mistydemeo / tigerbrew

Experimental fork of homebrew for PPC Macs on Tiger
Other
542 stars 130 forks source link

openssh: fixed the test #1142

Closed gsteemso closed 6 months ago

gsteemso commented 6 months ago

It had been using a method call from upstream Homebrew. I copied the few lines of actual meat into the test routine and it seems to work fine now.

sevan commented 6 months ago

Would it not be better to make the method available as part of framework so it can be used in other formulas?

gsteemso commented 6 months ago

It probably would, but I wasn't sure where to best put it short of slavishly imitating what upstream did, and for all I know they only did so as part of a larger set of changes I know nothing about.

This is one of a number of Tigerbrew-related things I would like to discuss with you, but I can find no way of doing so without first opening an error ticket or a pull request, in which case I am required to air my half-baked thought process to the world while it happens. Are you aware of any direct-messaging function within github?

sevan commented 6 months ago

I don't know of any DM facility. I'd take a chance with the issues section as others can contribute & it is there for the future should someone else coming along wondering the same thing. If it leads onto a pull request, the issue where the discussion took place can be referenced.

gsteemso commented 6 months ago

Upstream integrated this function as its own, very short, entirely discrete library file within the Homebrew directory. It is only referenced from a few places, most of them having to do with testing it.

If we choose a similar approach, I personally am inclined to stick it in utils -- either the file or the subdirectory, it doesn't really matter which. There are far too many library files at the root level already; cataloguing what calls what and when took me weeks, and I still haven't gotten into a lot of the less-frequently-used stuff.

While I'm on that subject, the amount of redundant and disorganized code spread through the main scripts is horrifying and I will be submitting patches at some point. I have two data files I've been amassing, respectively listing what actually happens when brew is invoked and what variables and utility functions are in fact available to formula authors / those who would otherwise edit Tigerbrew, and they are both a LOT longer than seems at all reasonable. (The main code path, the part that is executed on every invocation, wanders through a lot of territory that could probably be deferred until the need for it is confirmed; and several tasks are redundantly done twice, because nobody writing the Ruby portion thought to check whether the shell-script portion had already done them.)

gsteemso commented 6 months ago

Regarding the openssh test, I've further discovered that a lingering sshd configuration file from a (removed) previous install can also cause the test to fail, through no fault of the current installation. Is the package's failure, upon uninstallation, to remove the /usr/local/etc/ssh/* that it installed intentional?

sevan commented 6 months ago

Not sure, have you made changes locally to those files?

gsteemso commented 6 months ago

Not that I was aware of, although I think something else must have edited one of them because it contained UTF-8 text that choked sshd (that was, coincidentally, what it did to derail the test). However, the other two files appeared to have never been touched (admittedly, I have no way to know for certain).

sevan commented 6 months ago

I guess you could try to uninstall openssh, ensure that there are no files present by deleting any remains then doing an install, uninstall. Were there config files left at the end after all that?

gsteemso commented 6 months ago

The whole ${HOMEBREW_PREFIX}/etc/ssh/ directory, deleted wholesale before being freshly installed, was left untouched by brew rm. I think it might be because they were generated after the rest of the formula, with a process involving its own little resource download? I've not yet dug deeply enough into the resource-management mechanisms to have too clear a picture, there.

sevan commented 6 months ago

I tend to use brew uninstall I'm not sure if that's synonymous with rm though.

mistydemeo commented 6 months ago

Is the package's failure, upon uninstallation, to remove the /usr/local/etc/ssh/* that it installed intentional?

This is intentional! The user's free to customize the files in etc, so they're intended to survive uninstalls. It's not perfect though - like you mentioned here, there's always the possibility of version incompatibilities.

Thanks for this PR; I'll try to test it soon.

gsteemso commented 6 months ago

Sevan, brew rm and brew uninstall are the same command. I forget which is aliased to the other or whether they're both aliases of a third, but it's the exact same command being executed.

Misty, you’re absolutely right about version incompatibilities. It appears to have been a case of OpenSSL 3 stomping on OpenSSL 1’s configuration, and when I nuked the lot of them, I inadvertently blew away my Github access key! Whoops. Easily fixed but embarrassing.

Either of you, do you have any preference on where the free_port function should be put if it is made a general-use utility routine as Sevan suggests? I object on proliferation grounds to putting it in its own library file as upstream did, but obviously Misty has final say.

sevan commented 6 months ago

No objection from me, it was just a suggestion. I guess it can stay as is & be moved later should another formula need the functionality.

gsteemso commented 6 months ago

Makes sense to me! Marking this closed.

gsteemso commented 6 months ago

Whoops, forgot that was a pull request and not an error ticket. Please excuse the disturbance.