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] Loading only one helper file leads to errors because missing getopts #651

Merged
merged 1 commit into from Feb 18, 2019

Conversation

Projects
None yet
2 participants
@alexAubin
Copy link
Member

commented Feb 16, 2019

The problem

Ran a postinstall on unstable and realized that some regenconf hooks were broken because they use a helper, and getopts wasn't sourced...

Solution

Well that's a solution where I just load all helpers instead as in apps. Dunno that's what we want, maybe there's a significant overhead compared to sourcing getopts in each file ?

PR Status

Not tested

How to test

Run a postinstall with and without this branch

Validation

  • Principle agreement 0/2 :
  • Quick review 0/1 :
  • Simple test 0/1 :
  • Deep review 0/1 :
@maniackcrudelis

This comment has been minimized.

Copy link
Contributor

commented Feb 16, 2019

That's probably a better idea to load all helpers, considering helpers are more and more related to each others.

@alexAubin

This comment has been minimized.

Copy link
Member Author

commented Feb 16, 2019

Some numbers trying on a RPi 2 :

time for I in `seq 1 10`; do source /usr/share/yunohost/helpers.d/ip; done

-> 0.03 secs

time for I in `seq 1 10`; do source /usr/share/yunohost/helpers; done

->1.2 secs

So I guess the relative difference is huge, but in terms of absolute value I think it's not a big deal to add 1 sec to postinstall for this ...

@alexAubin alexAubin marked this pull request as ready for review Feb 16, 2019

@alexAubin alexAubin added this to the 3.5.x milestone Feb 16, 2019

@alexAubin

This comment has been minimized.

Copy link
Member Author

commented Feb 18, 2019

Yolomergin'

@alexAubin alexAubin merged commit 4ddfa68 into stretch-unstable Feb 18, 2019

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@alexAubin alexAubin deleted the fix-missing-getopts branch Feb 18, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.