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
554 stars 130 forks source link

Review the logic for setting the PATH with preferXpacksBin #580

Closed ilg-ul closed 11 months ago

ilg-ul commented 1 year ago

This issue was discussed in https://github.com/eclipse-embed-cdt/eclipse-plugins/pull/576.

For projects that use xpm/npm dependencies, the tools are local to the project, and the PATH should be set to guarantee that these local tools are used.

To test such configurations, the xPack Arm template can be used to generate projects with dual identity, as Eclipse projects and as standalone command line projects.

For more details, see Hello World Arm & RISC-V QEMU xPack templates.

ilg-ul commented 11 months ago

@jonahgraham, v6.4.0 is almost ready, this is the only pending issue.

Do you think we can solve it before 2023-09 closing date?

jonahgraham commented 11 months ago

@ilg-ul I'll try to come back to it, but more critical items have creeped in my todo list.

ilg-ul commented 11 months ago

What is the 2023-09 closing date? If things get that busy we can delay it for -12, but I'd prefer to have it in -09, to remove this item from my todo list, since I don't plan for a new release in the near future.

I might also try to address it myself if you explain me what the problem was, since I did not understand it from your previous PR. :-(

ilg-ul commented 11 months ago

I took a closer look at your explanations in #576, and, if I got it right, you identified a bug in the following sequence:

IPath projectPath = project.getWorkspace().getRoot().getLocation().append(project.getFullPath());
IPath xpackBinPath = projectPath.append("xpacks").append(".bin");

path.add(xpackBinPath.toOSString());

and suggested:

String xpackBinPath = project.getFolder("xpacks").getFolder(".bin").getLocation().toOSString();
path.add(xpackBinPath);

This should produce a correct path, even if the project is not stored in the workspace.

I tested this with a project created via the QEMU Arm xPack template outside the workspace and the build passed, so I guess this is ok.

ilg-ul commented 11 months ago

I did an attempt to fix this issue in commit https://github.com/eclipse-embed-cdt/eclipse-plugins/commit/2eea0558460dc4755af931d0c2d04c33221055dc.

Please review it and let me know if this is what you meant.

jonahgraham commented 11 months ago

String xpackBinPath = project.getFolder("xpacks").getFolder(".bin").getLocation().toOSString();

That is what I meant.

What held me up here is that I didn't take time to understand the other parts of your request, but seeing what you changed it LGTM

ilg-ul commented 11 months ago

Ok, thank you. Then I'll proceed with the release.