Introducing APP_DIR property #3421
Conversation
|
Build failed. |
|
AFAIU, the OSX bundler app already recognized the $APPROOT variable, but you just modified to not change the current directory, right? Any plans on submitting these changes upstream? And wouldn't this allow the --curdir handling to be removed? |
|
Oh, and kudos for diving into this! Thanks! |
|
The bundler knows about APP_ROOT but doesn't propagate it to the app. I've submitted the changes upstream: https://bitbucket.org/infinitekind/appbundler/pull-request/32/workingdir-set-to-nsfilemanager/diff Indeed this makes the --curdir hack useless. However, I'd leave it there for backwards compatibility |
|
I've removed the indentation change for a clearer diff |
|
Build failed. |
|
@ArduinoBot build this please |
|
Build failed. |
|
Whoops, didn't push |
I'm confused - it seems APP_ROOT is already interpolated into (among others) all option values, see this bit of code The only thing your patch seems to change is, when no "WorkingDirectory" is specified, do noop chdir (to the current directory) instead of to the home directory? (which is fine, just seeing if I understand this correctly)?
Compatibility for what exactly? People running java directly passing --curdir (seems unlikely). Or do you want to keep the code around in case we'll need it later? (seems tricky - if it's not needed it will likely bitrot and no longer work when you need it). In any case, I'd remove the --curdir option from the arduino shell script, that only complicates matters and potentially causes inconsistencies between platforms. Additionally, there seems to be a stray "w$" (and a missing full stop) in your commit message? |
|
No, that bit of code replaces occurrences of the You're right about my patch, expect I fear it's not a noop chdir. I'm not sure, but otherwise I wouldn't be able explain this code: if it was a noop, Compatibility with those users used to the bug and calling the IDE from the CLI setting CWD to an arbitrary value, different from the actual CWD I'll include removal of |
|
Turns out |
That's exactly what I meant when I said "AFAIU, the OSX bundler app already recognized the $APPROOT variable,". I think we agree, in any case.
It's called in the second case to chdir to $HOME, which is likely not the cwd yet, so it was not a noop previously. Or am I totally misunderstanding you here?
Yes, it was only meant as an option for internal use, never meant to be used externally. IMHO, anyone that was using it anyway (if any at all) are on their own, no need to keep messy code around for them... |
|
|
… folder. Also upgrading appbundler with a patched version: https://bitbucket.org/ffissore/appbundler It allows to know the current working directory These two put together, we can now rely on APP_DIR when loading IDE resources while being sure current working directory is properly set, thus being much more friendly when run from CLI Fixes #1493
It allows the IDE to know its installation folder.
Also upgrading appbundler with a patched version: https://bitbucket.org/ffissore/appbundler
It allows to know the current working directory
These two put together, we can now rely on APP_DIR when loading IDE resources while being sure current working directory is properly set, thus being much more friendly when run from CLI
Fixes #1493
/cc @wayoda @haobug @cmaglie @matthijskooijman
We need to make a windows installer as well, to be sure this doesn't break double clicking a
.inofile