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

fix: Installer improvements and bug fixes #223

Merged
merged 11 commits into from
Jun 28, 2023
Merged

Conversation

XavierChanth
Copy link
Member

@XavierChanth XavierChanth commented Jun 28, 2023

- What I did

  • Made the tar unpacking less verbose so the installer is cleaner
  • added logrotation to the non-tmux installation option which closes Introduce better logging for cron sshnpd mechanism #175
  • merged updater into installer
    • Checks if the service type is cron, and adds a logrotate line to crontab if it is
    • Checks to see if there was an existing cron entry for this service before trying to start sshnpd (i.e. only start if it is truly the first install)
      closes FileSystemException inside ~/.sshnp #203
  • Exits if the tar/zip file is missing
  • Escapes "." in ARCH so that the armv7l architecture only matches arm, not arm64

- How I did it

- How to verify it

- Description for the changelog
fix: Installer improvements and bug fixes

scripts/install_sshnpd Outdated Show resolved Hide resolved
@XavierChanth
Copy link
Member Author

@JeremyTubongbanua I've removed the mandatory tag

Copy link
Member

@JeremyTubongbanua JeremyTubongbanua left a comment

Choose a reason for hiding this comment

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

"options" might be confusing for some people maybe, I've seen them called flags

@XavierChanth
Copy link
Member Author

XavierChanth commented Jun 28, 2023

From what I've seen it called in the industry:
flags = no value after (--help, -u) are flags
options = value after (-c @‎ client, -d @‎ device, -n devicename) are options

essentially a flag is a boolean option

x86_64|amd64) ARCH="x64.";;
armv7l|arm) ARCH="arm.";;
riscv64) ARCH="riscv64.";;
aarch64|arm64) ARCH="arm64\.";;
Copy link
Contributor

Choose a reason for hiding this comment

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

you could also consider using single quotes

Copy link
Contributor

Choose a reason for hiding this comment

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

but if you're later using this in a double-quotes expression, probably best to leave as double quotes :-)

Copy link
Member Author

@XavierChanth XavierChanth Jun 28, 2023

Choose a reason for hiding this comment

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

This is a bug fix that @cconstab found, I want to match a "." at the end of the string using grep, but grep accepts regex. So I need the "\" in the string to escape the ".".

echo $ARCH outputs "arm64\." in this case.

The one causing issues was "arm." which was matching both "arm64." and "arm."

Copy link
Member Author

@XavierChanth XavierChanth Jun 28, 2023

Choose a reason for hiding this comment

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

e.g.

~/temp 
➜ ls
arm.tgz   arm64.tgz

# with the \

~/temp 
➜ test_ARCH="arm\."   

~/temp 
➜ ls | grep $test_ARCH
arm.tgz

~/temp 
➜ echo $test_ARCH
arm\.

# without the \

~/temp 
➜ test_ARCH="arm." 

~/temp 
➜ ls | grep $test_ARCH
arm.tgz
arm64.tgz

~/temp 
➜ echo $test_ARCH
arm.

Copy link
Member Author

Choose a reason for hiding this comment

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

you could also consider using single quotes

See line 173

@XavierChanth XavierChanth requested a review from gkc June 28, 2023 14:55
Copy link
Member

@cconstab cconstab left a comment

Choose a reason for hiding this comment

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

tested on armvl pi works fine

@XavierChanth XavierChanth merged commit fbfa8ec into trunk Jun 28, 2023
@XavierChanth XavierChanth deleted the Xavier-patches-3.0.0 branch July 6, 2023 15:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

FileSystemException inside ~/.sshnp Introduce better logging for cron sshnpd mechanism
4 participants