Avoid calling GPSBabel if the input and output are both gpx files

[huobos]

GPSBabel fails to import GPX file with spaces and Chinese in its name. It's better avoid calling it if the input and output are both GPX files.

[gdt]

Are you sure that the behavior will be identical in all cases? GPSBabel seems to also normalize and clean up gpx, so gpx->gpx transforms are not necessarily pointless. I really don't understand the details here, but the commit message does not explain enough for a reader to understand.

It seems better to fix GPSBabel.

[huobos]

The behavior is identical to opening the gpx file in Viking->File->Open directly. GPSBabel always fails with space and wide chars in input file name. I agree that shall be fixed in GPSBabel. On Jan 20, 2017 8:06 PM, Greg Troxel <notifications@github.com> wrote:Are you sure that the behavior will be identical in all cases? GPSBabel seems to also normalize and clean up gpx, so gpx->gpx transforms are not necessarily pointless. I really don't understand the details here, but the commit message does not explain enough for a reader to understand. It seems better to fix GPSBabel. —You are receiving this because you authored the thread.Reply to this email directly, view it on GitHub, or mute the thread. {"api_version":"1.0","publisher":{"api_key":"05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":{"external_key":"github/viking-gps/viking","title":"viking-gps/viking","subtitle":"GitHub repository","main_image_url":"https://cloud.githubusercontent.com/assets/143418/17495839/a5054eac-5d88-11e6-95fc-7290892c7bb5.png","avatar_image_url":"https://cloud.githubusercontent.com/assets/143418/15842166/7c72db34-2c0b-11e6-9aed-b52498112777.png","action":{"name":"Open in GitHub","url":"https://github.com/viking-gps/viking"}},"updates":{"snippets":[{"icon":"PERSON","message":"@gdt in #18: Are you sure that the behavior will be identical in all cases? GPSBabel seems to also normalize and clean up gpx, so gpx-\u003egpx transforms are not necessarily pointless. I really don't understand the details here, but the commit message does not explain enough for a reader to understand.\r\n\r\nIt seems better to fix GPSBabel."}],"action":{"name":"View Pull Request","url":"#18 (comment)"}}}

[rnorris]

GPSBabel handles files with spaces, but you'll need to quote files like "file name.gpx".
This happens transparently in Viking as we invoke GPSBabel via g_spawn_async_with_pipes () which does that for us.

However for 'exotic' filenames such as 'æ.gpx' or similar then possibly GPSBabel can fail.

I think this occurs for Qt4 builds of GPSBabel such as the current version 1.5.3 on Debian AMD64. I guess old versions pre the use of Qt (such as GPSBabel 1.3?) might work correctly.

However if you use the latest git head source code GPSBabel (which is now Qt5 only) it now works.

[huobos]

So this code is not neccesary.

[gdt]

I didn't mean to say I was sure it wasn't a good idea. Just that a change, especially to work around a bug that should be fixed, should clearly explain what the impact on behavior is.

When gpsbabel was a regular program, viking depending on it seemed fine. But now that it drags in qt5, I think we should reconsider. So the idea that gpx is the native format, and gpsbabel isn't used could make a lot of sense. Then, gpsbabel could be not really a dependency, and the other formats could use it if asked for, and warn if not installed.

Really what I am raising is the notion of having viking packages not depend on gpsbabel.

[rnorris]

GPSBabel is not an explicit dependency, just highly recommended.

[gdt]

Thanks. Then I will drop it from the viking package in pkgsrc. People who want it can install it. Presumably it doesn't need to be there at configure/build time.

[gdt]

And in that case, if the above patch or similar means that viking can open gpx files without gpsbabel, I think that would be a great improvement.

[rnorris]

Viking doesn't need GPSBabel at configure or build time.

It tries to detect it at runtime and stuff that would use it is generally disabled or at worst some kind of error message about the functionality not working correctly when the action is attempted.

I believe Viking has never needed GPSBabel to open GPX files, but since GPX is a format supported by GPSBabel you can force loading a GPX file via the File->Acquire->Import File with GPSBabel... route - should you desire to do so!

This patch probably would override that and so is not desired.

[gdt]

Thank you for the explanation. It then sounds like all is well code wise in viking, and it's ok for my package to not force gpsbabel.