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

cowsay -r: "Cannot load cow from /usr/share/cowsay/cows/TextBalloon.pm: .pm format cows are not implemented yet. Sorry." #22

Open
ndim opened this issue Jul 14, 2022 · 6 comments
Assignees
Labels
bug Somethin ain't right
Milestone

Comments

@ndim
Copy link
Contributor

ndim commented Jul 14, 2022

When running cowsay -r, occasionally cowsay fails with a message like

$ date | cowsay -r
cowsay: Cannot load cow from /usr/share/cowsay/cows/Frogs.pm: .pm format cows are not implemented yet. Sorry.
$ date | cowsay -r
cowsay: Cannot load cow from /usr/share/cowsay/cows/TextBalloon.pm: .pm format cows are not implemented yet. Sorry.
$ date | cowsay -r
cowsay: Cannot load cow from /usr/share/cowsay/cows/DragonAndCow.pm: .pm format cows are not implemented yet. Sorry.
$ _

I would expect cowsay -r to choose a random cow from all the available cows which actually work, and then use that.

Quick command to reproduce: while date | cowsay -r; do :; done

A look into /usr/share/cowsay/cows/ shows 7 *.pm files, one mech-and-cow file without a file extension, and a few dozen *.cow files.

Manually choosing a certain cow does not make the *.pm cows work either. The mech-and-cow (without file extension) cow does not work either, but triggers a different error:

$ date | cowsay -f MechAndCow.pm
cowsay: Cannot load cow from /usr/share/cowsay/cows/MechAndCow.pm: .pm format cows are not implemented yet. Sorry.
$ date | cowsay -f mech-and-cow
Backticks found where operator expected at /usr/share/cowsay/cows/mech-and-cow line 10, near "^__^                             /   /'|  `"
	(Missing semicolon on previous line?)
Backslash found where operator expected at /usr/share/cowsay/cows/mech-and-cow line 11, near ")\"
	(Missing operator before \?)
Backticks found where operator expected at /usr/share/cowsay/cows/mech-and-cow line 12, at end of line
	(Missing semicolon on previous line?)
cowsay: Error reading cow definition from /usr/share/cowsay/cows/mech-and-cow: Can't find string terminator "`" anywhere before EOF at /usr/share/cowsay/cows/mech-and-cow line 12, <STDIN> line 1.

$ _

And for completeness' sake:

$ cowsay --version
/usr/bin/cowsay version 3.7.0 calling Getopt::Std::getopts (version 1.13),
running under Perl version 5.34.1.
$ _
@apjanke
Copy link
Collaborator

apjanke commented Jul 15, 2022

I would expect cowsay -r to choose a random cow from all the available cows which actually work, and then use that.

Your expectation is correct. The random-cow-selection code should be fixed to do this.

@apjanke apjanke self-assigned this Jul 15, 2022
@apjanke apjanke added the bug Somethin ain't right label Jul 15, 2022
@apjanke apjanke added this to the 3.8.0 milestone Jul 15, 2022
@ndim
Copy link
Contributor Author

ndim commented Jul 16, 2022

I would expect cowsay -r to choose a random cow from all the available cows which actually work, and then use that.

Your expectation is correct. The random-cow-selection code should be fixed to do this.

If all cow files worked, the selection code could remain unchanged. :-)

AFAICT, that would mean implementing support for the *.pm type and either converting the non-working mech-and-cow to the *.cow format or removing mech-and-cow in favour of the existing MechAndCow.pm.

I have not examined the differences between .cow and .pm, so I do not know what one offers over the other, and whether .pm is a legacy format or the future.

@apjanke
Copy link
Collaborator

apjanke commented Jul 18, 2022

I believe the .pm cow format is the future (but as an optional alternative; the original .cow format will not go away and most people will still want to use it instead), and it allows you to build cows more dynamically, for whatever reason you might want to do that. But AFAICT, it never got implemented in the first place, and would take a bit of work, and I'm kinda busy. So that's why we don't have .pm support here yet.

Given that, I think the right move here is to exclude .pm cows from the random selection.

Unless you'd like to submit a PR with .pm cow support. ;)

@ndim
Copy link
Contributor Author

ndim commented Jul 18, 2022

I believe the .pm cow format is the future (but as an optional alternative; the original .cow format will not go away and most people will still want to use it instead), and it allows you to build cows more dynamically, for whatever reason you might want to do that. But AFAICT, it never got implemented in the first place, and would take a bit of work, and I'm kinda busy. So that's why we don't have .pm support here yet.

Given that, I think the right move here is to exclude .pm cows from the random selection.

And remove the broken non-extension mech-and-cow file as well.

Unless you'd like to submit a PR with .pm cow support. ;)

Hmm... Maybe. I have not done Perl in about 20 years, and I have never written anything of consequence in Perl like a script longer than a few hundred lines, much less an API to last.

I could give it a try and re-learn some Perl. Do not hold your breath for that, though. This is not mission critical after all. Also, do not expect very good Perl API design or very good Perl code.

On the other hand, keeping old and more or less non-essential but still interesting software maintained at least to the point of keeping it running on contemporary systems is just my thing.

Lets see. A categorical maybe.

@apjanke
Copy link
Collaborator

apjanke commented Jul 18, 2022

And remove the broken non-extension mech-and-cow file as well.

D'oh! Yeah, that too.

Hmm... Maybe. ... I could give it a try and re-learn some Perl.

I was 100% joking here. I would not recommend this as a project if you have anything else interesting to do. :)

@ndim
Copy link
Contributor Author

ndim commented Jul 18, 2022

As it has not been mentioned yet in this issue, issue #14 tracks implementing *.pm cows.

ndim added a commit to ndim/cowsay that referenced this issue Aug 14, 2022
Only install cow files called *.cow into the $(cowsdir)/.

This happens to fix cowsay-org#22
for default cowsay installation as the "cowsay -r" random cow
selection algorithm now can only select from *.cow format cows
from the default cowsay installation.

However, the site-cows/ dir can still contain arbitrary types of cow
files, and third party cow collections can still dump their cows into
cowsay's $(cowsdir)/ dir as well, so for those non-defaultinstallations,
issue cowsay-org#22 will *NOT* be fixed.
ndim added a commit to ndim/cowsay that referenced this issue Aug 14, 2022
Only install cow files called *.cow into the $(cowsdir)/.

This happens to fix cowsay-org#22
for default cowsay installation as the "cowsay -r" random cow
selection algorithm now can only select from *.cow format cows
from the default cowsay installation.

However, the site-cows/ dir can still contain arbitrary types of cow
files, and third party cow collections can still dump their cows into
cowsay's $(cowsdir)/ dir as well, so for those non-defaultinstallations,
issue cowsay-org#22 will *NOT* be fixed.

This also helps against unintentionally installing e.g. editor backup
files like "share/cows/default.cow~"
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Somethin ain't right
Projects
Status: Todo
Development

No branches or pull requests

2 participants