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 PWD directory of start script #3138

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

Tony763
Copy link
Contributor

@Tony763 Tony763 commented Oct 25, 2022

Description

Hi, I'm proposing a fix for issue #3137.

For getting directory use $PWD and for getting directory of script which was caller use $0.

How to test

Try to start Mycroft by calling mycroft-start debug and sh start-mycroft.sh debug

Contributor license agreement signed?

CLA [x] (Whether you have signed a CLA - Contributor Licensing Agreement

@devops-mycroft devops-mycroft added the CLA: Yes Contributor License Agreement exists (see https://github.com/MycroftAI/contributors) label Oct 25, 2022
@forslund
Copy link
Collaborator

Hmm, I see that the current code isn't quite working correctly. However the change doesn't seem to work if calling the script from outside the mycroft-core folder (which I believe was the intent of the original code).

@Tony763
Copy link
Contributor Author

Tony763 commented Oct 28, 2022

If I call mycroft-start debug from my home folder, it works normally.

Or You think calling start-mycroft.sh? Should it be called directly?

@forslund
Copy link
Collaborator

At least it worked previously.

A simple addition seems to be to add

cd $(readlink -f "$0" | xargs dirname)

before the setting DIR (the old version did something similar)

which will execute the script from the mycroft folder.

The mycroft-start duplicates this but in a bash-specific way.

@PureTryOut do you have any suggestions or comments?

@Tony763
Copy link
Contributor Author

Tony763 commented Oct 28, 2022

If I change cd -P "$( dirname "$SOURCE" )" into cd $(readlink -f "$0" | xargs dirname) in start-mycroft.sh then it can be called from every place. But If I call mycroft-start then it will point to the wrong directory (/srv/mycroft-core/bin). $0 will return path to first called script.

@PureTryOut
Copy link
Contributor

@PureTryOut do you have any suggestions or comments?

Well my suggestion would be merging #2802 which changes large parts of the script anyway. 😜

The original code wasn't mean to make it possible to run the script from everywhere persé. As far as I can see it will always cd to where the script is located (the real location, not any symlink). I suppose the bin/mycroft-start use-case hasn't been tested very well, but with #2802 I was getting rid of the duplicated entry point anyway.

I'm not entirely sure what to do here besides just getting my PR in, having 2 differently located and named entry points is a bit of a strange situation anyway.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA: Yes Contributor License Agreement exists (see https://github.com/MycroftAI/contributors) hacktoberfest-accepted
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants