#582 target specific custom variables not reinitialized for different build target runs

[Jannick]

A target specific custom variable of a prior build run appears not to be re-initialized, but rather kept unchanged if not defined by the current target.

Reproduce using the simple example in the zip attached:

1. Remove obj and bin folder if they exist
2. Build target 'Debug' (which echos - as a pre-build action - the content of the custom variable FLAG defined for target 'Debug', but not for the target 'Release')
3. Build target 'Release' (which echos the same string as in (2) albeit not defined for 'Release'; expected that the variable is reinitialized to the empty string)

Cf. result in compilation log 'BuildLog.txt'

[bluehazzard]

I can confirm and will inwestigate this, but what is the expected result?
Failing the build, because not all is defined? Filling with whitespace?

[Jannick]

Filling with an empty string. This is the default behaviour when, e.g., running (1), then (3) of the example for a freshly fired instance of CB.

[bluehazzard]

Ok i have looked again. I am not quite sure how to solve this.
My first idea was to delete all macros and recreate them at every build step. But as far as i understand this, the build process starts at the top (project level) creates all macros and propagates to bottom, If we deltete dhe macros on every build step the project macros will get lost...
I am not familiar with the build process code, and need some time to understand all this...

[bluehazzard]

Yea, i am pretty sure, that there is no way to know what macros (macros are called all variables that are replaced somewhere in the build process) are from a previous target or project. The source is not saved. So one possibility would be to clear all macros from one target after the build process. But this mean large work on the build process and i don't think the devs are happy with this. Any ideas?

[Teodor Petrov]

Shouldn't it be possible to modify MacrosManager::RecalcVars to set and reset the pre-target custom variables? You could remember their old values in some map and just reset them to their values in the map when the target changes; then you could set the new values for the new target.

Or you could store the values in a map with key the pointer of the target.

[bluehazzard]

Ok, here is (the eventual first version of) a patch.
I tried first the m_LastTarget variable to remove the last target from the macros. But this did not work, not quite sure why...

[Teodor Petrov]

1. why are you using a set? unordered_set should be a little faster?
2. what is tit? better name please!
3. why there is a loop?
4. isn't this an infinite one if *tit == target?
5. what happens if the same variables are set in the project?

[bluehazzard]

Ok, here is version 2:
i am not sure why m_LastTarget did not work on first try, but it should always be valid and we can use it to remove the last target macros. There is no other place the macros get changed so this should be save. So i removed the whole set thing.

1. what happens if the same variables are set in the project?
   They are re added after.. Nothing has changed here.

[Teodor Petrov]

You should give better explanation...

What happens if you have a project:
1. with 2 targets
2. one virtual target which merges the to targets.
3. there is a variable X set on the project and in target1
4. X is not set in target 2

So what happens when you build the virtual target?

[Jannick]

Confirming that patch v2 remedies the issue of this ticket. Would be happy if this could go into the next release / nightly build.

[bluehazzard]

Best way to find out is to read the code in macrosmanager.cpp:217 MacrosManager::RecalcVars()
I can describe it in 1000 words, but the code will tell you more than my broken English...

The current (buggy) way:
1) If current project != last project reset all project macros to new project
2) Store all project variables in the m_macros variable
3) Store all target variables in the m_macros variable
4) If target and project have the same variable, the project variable gets overwritten from the target variable

The new way
1) If the last target was not a nullptr remove all variables of the old target from the m_macros
2) If current project != last project reset all project macros to new project
3) Store all project variables in the m_macros variable
4) Store all target variables in the m_macros variable
5) If target and project have the same variable, the project variable gets overwritten from the target variable

The variables are getting read every time you call RecalcVar, so they are added every time in the correct order. Even if we delete a project variable, if she is in the current project she will get re-added...

i tried to make a project like you described and with this patch the result is as expected
For target 1 the variable of the target 1 is used
For target 2 the variable of the project is used

With the current code:
For target 1 the variable of the target 1 is used
For target 2 the variable of the target 1 is used

so clearly wrong, or am i missing something?

[Teodor Petrov]

I'm reviewing this patch again and I'm not sure I like the idea that the removal happens at the beginning if there is just a last target.

Isn't it better to do this removal when m_LastTarget is set to the value of target? This guarantees that your code won't be executed too often leading to performance problems.

Keep in mind that the macro manager is important for good performance of building and sometimes loading projects!

        Manager::Get()->GetLogManager()->Log(F(wxT("Cleaning marco manager for target %s"),
                                               m_LastTarget->GetTitle().wx_str()));

I've put the above code in the new if statement and tried to build the codeblocks-unit.cbp. After every target I get a lot of spamming (every target is mentioned at least once). Are we sure this is the expected and correct behaviour?

[bluehazzard]

I think this is a problem for a other ticket. As far as i can tell this is done in the

void CompilerCommandGenerator::Init(cbProject* project)

function, that is called for every target multiple times (3 times for example for the CB project). And every time all targets of the project are recalculated...

this is probably a lot wasted time... But i am not deep into the whole build process....

[Teodor Petrov]

You are sort of correct, so I'll push this... Lets see how many people complain that we've broken their builds. :)

[bluehazzard]

related xkcd: https://xkcd.com/1172/
Should i open a ticket for the performance issue?

[Teodor Petrov]

Only if you intend to provide a fix for it. I know about it, I don't need a ticket. :)

[Miguel Gimenez]

This change makes C::B fail with a SIGSEGV:

Thread 1 received signal SIGSEGV, Segmentation fault.
0x01fbde9c in MacrosManager::RecalcVars (this=0xec93600, project=0xa8f5428, editor=0xa8a6b70, target=0xa9ce5a0)
    at G:\Codeblocks\src\sdk\macrosmanager.cpp:229
229             const StringHash& v = m_LastTarget->GetAllVars();

To reproduce:
- Open a project
- Close the workspace
- Open the same project
- C::B fals with a SIGSEGV

I have attached the backtrace

[Teodor Petrov]

I've just hit the same segmentation fault today...

[Teodor Petrov]

Should be fixed in trunk now. Please let me know if you see further problems.

[Jannick]

1+ ... work fine now with nightly Oct 2018 (on Win10). Happy to see this ticket closed now.

Many thanks!