Skip to main content

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [cdt-dev] Library names with spaces fail

Op 13/09/2019 om 13:51 schreef Liviu Ionescu:

On 13 Sep 2019, at 14:30, Jan Baeyens <jan@xxxxxxxxxx> wrote:

... The -i test Livius proposes is in the current code the most obvious way. But ... will it cause regression... nobody knows.
I'm not proposing this path, it was just a hack to temporarily fix my tests, and I suggested that more acknowledgeable people study the problem and find a better solution.
Sorry, It should have said:

The -i test Livius uses in his workaround is IMHO a way compliant with the rest of the code.

I think it would be better to work in multiple steps.
The first step: convert the project to a "make oriented model" based on org.eclipse.cdt.managedbuilder.core.buildDefinitions.
The second step: Convert "make oriented model"  to make files
that would be nice, but it looks like it is very elaborate, and I'm afraid there will be no volunteers to do it.
It is very elaborate and for sure no solution for your problem nor a solution in the short run. However there are more issues with GnuMakefileGenerator that could use a fix. It is up to the CDT core team to decide whether there is a future for a make based external build in CDT. If the message is that GnuMakefileGenerator will be deprecated there will be no volunteers and quick and dirty fixes are the only way to go.


a more realistic approach for this case would be to identify the specific issue and fix it on the spot.

and, in my opinion, the issue is that `ensurePathIsGNUMakeTargetRuleCompatibleSyntax()` is called on a string that is not a path, but also includes the '-l' prefix.

if we can move the ensurePathIsGNUMakeTargetRuleCompatibleSyntax() upper, at the point where the list is first constructed, and later append the '-l' to the result, the issue would not surface.

however I have no idea if this can cause regressions. for libraries probably not, but there are other use cases that need to be considered.
Currently the regression tests compare the make file outputs of a couple of toolchain setups. I have been stung by these regression tests failing correctly because I had overlooked one or more use cases. Maybe you are lucky and the change you propose will not hit the regression tests nor reality.

IMHO The right thing to do now is to deliver the patch you described and see what the regression tests say.



Back to the top