jcelaya / hdrmerge

HDR exposure merging
http://jcelaya.github.io/hdrmerge/
Other
355 stars 78 forks source link

AppImage release cwd is not correct -> problems with relative paths #159

Closed derMart closed 5 years ago

derMart commented 5 years ago

Hi I am using this release: https://github.com/jcelaya/hdrmerge/releases/download/nightly/hdrmerge-git-20180705.glibc2.14-x86_64.AppImage under Ubuntu 18.04. When specifying a relative path to an input file while using the command line mode, hdrmerge is not able to find it. I have to specify an absolute path.

aferrero2707 commented 5 years ago

I've started to work on a proper fix, which consists in removing the need of cwd change when the AppImage is launched.

aferrero2707 commented 5 years ago

@derMart I have prepared a new package, which does not change the CWD and therefore should accept relative paths like the standard HDRMerge command. Could you please check it? It can be downloaded from the nightly builds release as usual.

Beep6581 commented 5 years ago

Hi @aferrero2707 , if you would like a second pair of eyes to review the change, could you point me to the code change please?

aferrero2707 commented 5 years ago

@Beep6581 in fact, the only relevant change is this one. It removes a cd command in the main AppImage launcher script. All the rest of the modifications were needed to allow building the AppImage inside a docker container, for testing purposes.

No change was needed at the level of the HDRMerge code.

So I would say that the most important thing is to test the new AppImage and see if the removal of the CWD change has some bad side effect that I could not spot...

aferrero2707 commented 5 years ago

@Beep6581 I committed the changes you suggested regarding the quoting of variables. Thanks!

derMart commented 5 years ago

@aferrero2707 I can confirm, that the new nightly release works with relative paths, both for input as well as for -o output files :+1: Thank you very much!

aferrero2707 commented 5 years ago

Closing this since the problem seems to be fixed. Feel free to re-open the issue if something still does not work as expected...