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

script fixes #64

Closed
wants to merge 1 commit into from
Closed

script fixes #64

wants to merge 1 commit into from

Conversation

Salamandar
Copy link

Small script fixes.

I know the systemctl daemon-reload issue will be fixed in conf_regen, but calling it once more may prevent any warnings.

if is_raspbian ; then
# Reboot should be done before postinstall to be able to run iptables rules
info "Your server will now reboot. Please log back in as root and run the post-install."
reboot
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we can add a small delay like sleep 5 here ? (Or even 10 ?) Otherwise the user will see a bunch of messages with no time to read them which is always frustrating ...

Also wondering if it makes sense to call conclusion if we're on raspian but idk

Copy link
Author

Choose a reason for hiding this comment

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

@alexAubin Agreed for the sleep 5.
I think the conclusion is really important : without it, you're left wondering what to do after the reboot…

Copy link
Member

Choose a reason for hiding this comment

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

Hmyup my concern is more that people will probably not have the time to read it just in 5 sec ... Maybe the best UX would be to press a key (or enter Y) to agree to reboot after reading the whole thing

Copy link
Author

Choose a reason for hiding this comment

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

Well you don't need to. The text stays after ssh log-out.

…unless you're using some ssh client that clears the screen after disconnection.

Yeah, a "Press any key to reboot" would be great.

Anyways, could this script be automatically executed automatically at ssh connection ? Or at least, a
echo "Welcome, the yunohost install is not finished, please execute this command: …" could be shown.

@Salamandar
Copy link
Author

Salamandar commented May 29, 2020

Something like that could be printed on login :
Capture d’écran du 2020-05-29 18-01-35

This is done with a file /etc/profile.d/yunohost_post_install.sh, that could be simply removed when the post-install setup is completed.

EDIT: Here, I already ran the post-install, so the admin user exists, but imagine there's root instead and no sudo.

@alexAubin alexAubin closed this Jan 9, 2021
@alexAubin alexAubin deleted the branch YunoHost:buster-unstable January 9, 2021 21:58
@Salamandar
Copy link
Author

@alexAubin Reason for the close ? :(

@alexAubin
Copy link
Member

Hmpf sorry i was just cleaning up a bunch of old branches tonight and indirecly closed the PR ... re-reading i'm confused what this PR is about ... the thing about daemon-reload is now fixed in the core and hmpf idk about the reboot thing for RPi ... people are just supposed to use the preinstalled image (and what happens here is in fact what's used to build the preinstalled image) ... What you mention about "printing stuff on login" is I believe done / handled elsewhere via the yunoprompt stuff ... or there's also this which could be integrated in the install_script but that raises more question that it solve ... Again, people are just supposed to use the preinstalled image ... I'm just lost in what we're trying to solve here

@Salamandar
Copy link
Author

@alexAubin there aren't preinstalled images for all platforms. I have an ARM server that is supported by the Armbian project, but not by Yunohost I think.
And with the install script, the machine reboots without explanation, we don't know with which user we need to log in after the reboot (admin or root or…? ). As a matter of fact the documentation is not precise about that (I had issues about that when i installed my server this summer).

For the daemon-reload thing, alright. This merge request is only about printing more info to the maintainer during the installation process.

@alexAubin
Copy link
Member

alexAubin commented Jan 10, 2021

Indeed, but the changes in this PR were about is_raspbian, so only relevant for RPi ...

But yeah, the experience could be improved for boards for which we don't have pre installed images, so we could integrate https://github.com/YunoHost/arm-images/blob/yunohost/userpatches/overlay/check_yunohost_is_installed.sh in the install script somehow ... There's also some upcoming improvement to the documentation that may help YunoHost/doc#1491

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