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

More helpful error if missing lsb_release #288

Closed
wants to merge 1 commit into from
Closed

More helpful error if missing lsb_release #288

wants to merge 1 commit into from

Conversation

13r0ck
Copy link

@13r0ck 13r0ck commented Jan 27, 2024

Just getting started with pony lang for the first time today! Hopefully more actually helpful commits soon!

@ponylang-main ponylang-main added the discuss during sync Should be discussed during an upcoming sync label Jan 27, 2024
@SeanTAllen
Copy link
Member

What problem does this solve?

@13r0ck
Copy link
Author

13r0ck commented Jan 27, 2024

I was trying to create a pony project while developing in a distrobox container. Most containers do not come with lsb_release by default, so the install fails. Just makes the error more obvious

@SeanTAllen
Copy link
Member

The case statement isn't an appropriate place to check for that. It would be appropriate to check to see if lsb_release is installed before the command is executed and report back if it isn't. But at that point we are getting into other issues as well such as "what if curl isnt installed?" Having a check for one dependency being missing, but not others doesn't feel great to me.

What did you get as output from running the init script before you installed lsb_release?

I think at minimum it makes sense that we document what tools are required for the script to run. And perhaps before running any command, we check for its existence and print a message if it isn't installed (although that's not particularly "unix-y", but i'm not worried about being "particularly unix-y").

I've used an awful lot of base images for containers and they have all had lsb_release. What is the set of containers that you use that don't contain lsb_release?

@13r0ck
Copy link
Author

13r0ck commented Jan 27, 2024

I tend to agree with all of your points above. Just didn't want to step in suggesting other than the bare minimum for my first PR to the project :)

$ distrobox create temp --image fedora:latest
$ distrobox enter temp
$ sh -c "$(curl --proto '=https' --tlsv1.2 -sSf https://raw.githubusercontent.com/ponylang/ponyup/latest-release/ponyup-init.sh)"
sh: line 94: lsb_release: command not found
~~ SNIP ~~

I guess it doesn't fail for lsb_release reasons explicitly, but later it does fail without me setting ponyup default x86_64-unknown-linux-gnu, and I mixed those two up in my head.

But a quick check for missing dependencies might be nice anyway!

@SeanTAllen
Copy link
Member

FYI: ponyup default x86_64-unknown-linux-gnu was deprecated a while ago so, those will be some old compiler versions. Are you planning on using Pony with a particular Fedora where having a prebuilt compiler would be helpful?

@13r0ck
Copy link
Author

13r0ck commented Jan 27, 2024

Ah that makes sense why I was having build issues with that. Thank you for the heads up. A deprecation notice for that might be helpful?
I did manage to get ponyc working by installing via the nix package manager. It looks like that one is old too though. I can make myself useful and update that over there. Sorry for this becoming a longer thread than intended. We can close this, but if you would like me to try opening a PR for either a deps check and/or a deprecation warning on the unknown-linux-gnu, please let me know!

@13r0ck 13r0ck closed this Jan 27, 2024
@ponylang-main ponylang-main added discuss during sync Should be discussed during an upcoming sync and removed discuss during sync Should be discussed during an upcoming sync labels Jan 27, 2024
@13r0ck 13r0ck deleted the learnding branch January 27, 2024 22:05
@SeanTAllen
Copy link
Member

@13r0ck a deprecation notice would be nice. would welcome a PR for that.

is there a particular fedora you are looking to target? if there's a specific version we could look into adding builds for it. i'd need some assistance from you as i dont know fedora well.

@13r0ck
Copy link
Author

13r0ck commented Jan 27, 2024

Will do!

When I am using fedora it is usually in a distrobox container using fedora:latest because I want to do something else that requires newer packages than what ubuntu's repos have. Very much willing to help if I can, but I personally am not looking for a hard and fast prod environment yet

@SeanTAllen
Copy link
Member

Ok, so, would Fedora 39 work? Given that is what latest is now?

If yes, then what would be needed is the Fedora 39 based version of this Dockerfile:

https://github.com/ponylang/ponyc/blob/main/.ci-dockerfiles/x86-64-unknown-linux-ubuntu22.04-builder/Dockerfile

In particular, the software installation bits. If you can get me a Dockerfile that should match, I can try it out locally as an environment to test and build ponyc in.

Are you on the Zulip? If you are up for helping with the Dockerfile, then coordinating in the zulip here https://ponylang.zulipchat.com/#narrow/stream/190359-ci in the ci stream would be the easiest.

@13r0ck
Copy link
Author

13r0ck commented Jan 27, 2024

Will do! I will also move this chat over to zulip once I get that working. Thanks for the chat today, I hope you have a great rest of your weekend!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discuss during sync Should be discussed during an upcoming sync
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants