YunoHost-Apps / hotspot_ynh

Wifi Hotspot app for YunoHost
GNU Affero General Public License v3.0
39 stars 19 forks source link

Update scripts to the new standard #51

Closed kay0u closed 5 years ago

kay0u commented 5 years ago

The purpose of this PR is to update all scripts to the new standard and fix the main issues (Backup, Restore, Upgrade).

Reviewers have to test it on a brique (sorry, I don't have one :-))

kay0u commented 5 years ago

A very good thing to do is to move all scripts used from /usr/local/bin to /opt/$app/bin but I don't know if it's possible

kay0u commented 5 years ago

Package_checked. I think it's ready for review.

I can add the change_url script if you want?

kay0u commented 5 years ago

The big problem is that the upgrade is currently broken. I don't know how to deal with it.

But yeah, we have to test it on differents hardwares

keomabrun commented 5 years ago

Just tried installing on a lime2 and it failed :(

https://paste.yunohost.org/raw/sehanutoho

Here is the log line (in /var/log/syslog) that might be the culprit:

ynh-hotspot.service: Failed at step EXEC spawning /usr/local/bin/hotspot: No such file or directory
kay0u commented 5 years ago

Just tried installing on a lime2 and it failed :(

https://paste.yunohost.org/raw/sehanutoho

Here is the log line (in /var/log/syslog) that might be the culprit:

ynh-hotspot.service: Failed at step EXEC spawning /usr/local/bin/hotspot: No such file or directory

Could you retest it please?

keomabrun commented 5 years ago

Tested on a lime2 and it works.

keomabrun commented 5 years ago

We need to test the upgrade from a previous commit. Perhaps upgrade=1 from_commit=4f4f714bb6861f30aeaea6ffe0a97a973ef2633c ?

I believe it will fail the upgrade as you call yunohost service remove while its not sure it was added to yunohost before. Maybe use the following helper: https://github.com/labriqueinternet/vpnclient_ynh/blob/3dd730607be27d4fa011ae6c5d46fe83c6c5a9f2/scripts/_common.sh#L198

Cf: https://github.com/labriqueinternet/vpnclient_ynh/blob/3dd730607be27d4fa011ae6c5d46fe83c6c5a9f2/scripts/upgrade#L99

kay0u commented 5 years ago

In fact, I think we don't have to call "remove". If we call two times yunohost service add, the first will be erased by the second.

alexAubin commented 5 years ago

Indeed ! There's doesn't seem to be anything prevent it to run twice / override an existing service with yunohost service add

kay0u commented 5 years ago

We need to test the upgrade from a previous commit. Perhaps upgrade=1 from_commit=4f4f714bb6861f30aeaea6ffe0a97a973ef2633c ?

Upgrade from an older commit now pass (I used this one: 546fd17712c7a9df428a4f26e7f8e4394884aaaf instead of this one: 4f4f714bb6861f30aeaea6ffe0a97a973ef2633c because this last failed to install, due to firmware_nonfree. That was fixed by 546fd17712c7a9df428a4f26e7f8e4394884aaaf)

keomabrun commented 5 years ago

The install does not work due to the last commit https://github.com/labriqueinternet/hotspot_ynh/pull/51/commits/fcfe018a740515b1cde6b7351b8c4e8c3eb10645 using 'no' and not 0.

kay0u commented 5 years ago

Should be fixed now :-)

alexAubin commented 5 years ago

(Just passing by to say that I don't have so much time on my hands right now but I'm not forgetting that we should work on that PR /o)

keomabrun commented 5 years ago

I launched an install this morning on a lime2. Will keep you posted

keomabrun commented 5 years ago

Looks like it was in fact a hypercube issue: https://github.com/labriqueinternet/build.labriqueinter.net/pull/59

kay0u commented 5 years ago

Will be merged in 3 days! \o/