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

Updated MHL to run from same directory as mhl script #110

Merged
merged 2 commits into from
Mar 26, 2013
Merged

Updated MHL to run from same directory as mhl script #110

merged 2 commits into from
Mar 26, 2013

Conversation

PrimusNZ
Copy link
Contributor

@PrimusNZ PrimusNZ commented Mar 7, 2013

Updated MHL to allow both absolute and relative paths

E.g.
Instead of running:
cd /opt/misterhouse/bin
./mhl

You can now run:
/opt/misterhouse/bin/mhl
OR from the misterhouse root directory:
./bin/mhl

@peloy
Copy link
Collaborator

peloy commented Mar 8, 2013

Hi Ryan,

On 03/07/2013 06:37 PM, Ryan Davies wrote:

Updated MHL to allow both absolute and relative paths

E.g.
Instead of running:
cd /opt/misterhouse/bin
./mhl

You can now run:
/opt/misterhouse/bin/mhl
OR from the misterhouse root directory:
./bin/mhl

+# Set current working directory to location of this script (bin/)
+cd "$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
+

This only works if /bin/sh is bash. On some distributions, /bin/sh is a
POSIX-compliant shell, like dash, that does not support some of the
extra features supported by the Bash shell.

Applying this patch means that mhl would break for those using a
non-Bash /bin/sh.

In addition, the directory where mhl resides could be read-only.

I'd change mhl so the temp. files are written to another directory, like
MH's data_dir or /tmp, and so the mh binary is run with an absolute path
instead of just "perl mh" as it is currently being run by mhl.

Disclaimer: I don't use mhl.

Cheers,

Eloy Paris.-


    You can merge this Pull Request by running

git pull https://github.com/ProGEEK/misterhouse mhl

Or view, comment on, or merge it at:

#110

    Commit Summary

@PrimusNZ
Copy link
Contributor Author

PrimusNZ commented Mar 8, 2013

In theory replacing ${BASH_SOURCE[0]} with $0 should work, i'll update the script

@peloy
Copy link
Collaborator

peloy commented Mar 8, 2013

On 03/07/2013 07:25 PM, Ryan Davies wrote:

In theory replacing ${BASH_SOURCE[0]} with $0 should work, i'll update
the script

Yes, I've tested and that works with Dash.

Cheers,

Eloy Paris.-

@PrimusNZ
Copy link
Contributor Author

PrimusNZ commented Mar 8, 2013

Ive comitted the change.

I agree that the mh startup file needs to be written elsewhere, however i
dont know how we would correctly get the MH data dir, as it might not
always be a full path, Might be worth looking at switching it to write to
/tmp for now

R

On Fri, Mar 8, 2013 at 2:32 PM, Eloy Paris notifications@github.com wrote:

On 03/07/2013 07:25 PM, Ryan Davies wrote:

In theory replacing ${BASH_SOURCE[0]} with $0 should work, i'll update
the script

Yes, I've tested and that works with Dash.

Cheers,

Eloy Paris.-


Reply to this email directly or view it on GitHubhttps://github.com//pull/110#issuecomment-14597785
.

@krkeegan
Copy link
Collaborator

krkeegan commented Mar 9, 2013

Eloy, I agree with your concerns that the directory may be read only. However, am I correct in that the read only problem exists even absent this patch? If so I think we can commit this and note that issue for a future patch.

@peloy
Copy link
Collaborator

peloy commented Mar 9, 2013

On 03/08/2013 08:23 PM, krkeegan wrote:

Eloy, I agree with your concerns that the directory may be read only.
However, am I correct in that the read only problem exists even absent
this patch?

Yes, correct.

If so I think we can commit this and note that issue for a
future patch.

Sounds good!

Cheers,

Eloy.-

krkeegan added a commit that referenced this pull request Mar 26, 2013
Updated MHL to run from same directory as mhl script
@krkeegan krkeegan merged commit f6fefa5 into hollie:master Mar 26, 2013
@PrimusNZ PrimusNZ deleted the mhl branch March 26, 2013 04:23
@hollie hollie mentioned this pull request Jun 15, 2013
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.

3 participants