eclipse-embed-cdt / eclipse-plugins

The Eclipse Embedded CDT plug-ins for Arm & RISC-V C/C++ developers (formerly known as the GNU MCU Eclipse plug-ins). Includes the archive of previous plug-ins versions, as Releases.
http://eclipse-embed-cdt.github.io/
Eclipse Public License 2.0
555 stars 130 forks source link

Add PATH to pyocd search path default preferences #467

Closed flit closed 3 years ago

flit commented 3 years ago

This simple change makes it possible to find pyocd on PATH with only default preferences.

The older implementation that was removed in the last PR tested the resolved path, in the case it didn't find a file there, by calling out to pyocd. Then it would again call out to get probes and targets. That added an unnecessary additional external process invocation.

ilg-ul commented 3 years ago

I don't know what to say, by design all other plug-ins explicitly recommend to do not use the system PATH to find tool, since in this way it is not possible to control which version is used.

Instead, in Embedded CDT it is strongly recommended for all tools to be distributed as archives with versioned folders, which allow to easily install multiple version on the same system.

For consistency reasons I would prefer pyocd to somehow follow the same recommendations.

FYI, xPacks fully automate the process of installing such tools and the selection of the path is done by xPack version.

flit commented 3 years ago

Yeah, I generally agree! However, there was an issue #102 created John Cortell who wanted to ~use~ support use of pyocd on PATH. The last PR had a regression in that it broke that "feature", so this was a quick method of fixing it.

Maybe what's different from other plugins is that the pyocd plugin explicitly checks for a valid resolved path before calling out to pyocd to gather data. (If the path is found to be invalid, an appropriate error is added to the launch config dialog.) This means that the pyocd plugin (as of the last PR, with the regression in place) actively disallows PATH-resident pyocd: the pyocd path in the launch config must resolve to a valid file.

Ultimately this PR is just trying to avoid breaking a behaviour that some users may have come to expect. Dunno.

If you're comfortable saying tools on PATH aren't supported by Embed CDT, then I'm all for it and we can close this PR. 😄

ilg-ul commented 3 years ago

if you're comfortable saying tools on PATH aren't supported by Embed CDT

I'm definitely comfortable saying that my tools on PATH are strongly discouraged. I can do nothing to prevent some users to add the tools to the PATH, but the documentation explicitly recommends to install everything in the HOME folder and do not add them to the PATH; xpm does this automatically, and the actual path is quite discouraging to add manually to the PATH.

So the issue is not technical but administrative, even philosophical.

For your tools it is up to you to define the install strategy. Personally I'd prefer pyocd to also be available as an xPack, but if you prefer to install it in system locations, then probably this PR is useful.

flit commented 3 years ago

Imo, it's more about what the users want and not so much my desires. Though I do personally prefer system install locations (/opt or /usr/local) with appropriate PATH management, since I do a lot of development via command line. 😄

I'll be happy to create an xPack for pyocd once a binary build is available. (Mostly I need time to either find a solution to pyInstaller's issues with one of the dependencies, or try out pyOxidizer. And then get it building via CI for the major OSes.)

ilg-ul commented 3 years ago

I suggest you take a look at the meson binary xPack, it is a Python application that packs a Python instance and behave like a binary application (since it is in fact a binary application).

If you are interested in such an approach, we can discuss the details separately.


I also do a lot of command line development, but all my major tools are managed by xpm, so I don't have to deal with paths, xpm manages them automatically.

If you did not try it, do the following:

mkdir xxx
cd xxx
xpm init
xpm install -D -v @xpack-dev-tools/openocd@latest
ls -l xpacks/.bin

Then you can either add xpacks/.bin to your project path, or add the command line invoking openocd in package.json, as a named action, and invoke it with xpm run name, and in this case xpm knows the path automatically.

(to install xpm, please check https://xpack.github.io/install/)

So if you want to stick to system paths, we'll merge this PR, if you want to avoid system paths, and possibly use xPacks, we better don't merge it, since from my experience, it might be a source of trouble, especially if future releases of pyocd will introduce functional incompatibilities.

flit commented 3 years ago

I'm closing this PR. I realized that since the path code always looks in an appended bin directory it won't find anything on PATH. A better solution would be more like the original code that was removed.

ilg-ul commented 3 years ago

the path code always looks in an appended bin directory

Yes, this is the default package structure that the plug-ins recognise. All my binary packages use this structure.