Removed using namespace std from header files and fix errors

[lobis]

This is the smallest PR (that I could think of) that removes using namespace std; from all headers files and fixed all compilation errors.

It must be merged with the associated PRs from all libraries / packages which appear mentioned below.

Even though this PR should not have any effect on the framework, it may produce some unintended effects with some classes introduced by @nkx111 regarding endl in TRestStringOutput.h.

The previous implementation relied on having endl pointing to std::endl by using namespace std on the header. Now you cannot use endl on cout statements (because of missing namespace, unless you include it) and you cannot use std::endl on the metadata << type statements, because of this "collision". I think this should not be the case and should be fixed.

PR that also have to be merged along this one:

• Removed using namespace std from header files and fix errors connectorslib#9
• Removed using namespace std from header files and fix errors tracklib#14
• Removed using namespace std from header files and fix errors geant4lib#39
• Removed using namespace std from header files and fix errors detectorlib#43

[lobis]

For some reason pipeline https://gitlab.cern.ch/rest-for-physics/framework/-/jobs/20613842 is failing, but looks like its not using the corresponding version of the raw library, I think it should not fail.

From error https://gitlab.cern.ch/rest-for-physics/framework/-/jobs/20614036#L726 its obvious its not using the correct branch 🤔

[Vindaar]

What's the reasoning behind the sudden change? Just the fact that REST barely uses any STL stuff and is essentially a ROOT project instead?

Note that I do agree that pulling in std everywhere is rather problematic in C++.

[lobis]

What's the reasoning behind the sudden change? Just the fact that REST barely uses any STL stuff and is essentially a ROOT project instead?

Note that I do agree that pulling in std everywhere is rather problematic in C++.

It's part of our goal of cleaning up old bad practices, it's something we wanted to do for a while and now it's finally done ✅, it didn't take that much effort. There wasn't a change on design philosophy, I don't think there was a reason for it to be that way in the first place.

It makes no sense to use standard namespace in the header files even if with our current usage is not that problematic.

REST is supposed to be a framework you can integrate into other projects (or so I think, even though in practice it's not the case) and having to carry over this namespace is a big (potentially unavoidable?) problem as it could collide with some other library using the same namespace (which I guess would also be a bad practise but...).

Here is a more detailed explanation for anyone interested. https://stackoverflow.com/questions/14575799/using-namespace-std-in-a-header-file

[juanangp]

Do you understand why we have to revert std::endl to endl

[lobis]

Do you understand why we have to revert std::endl to endl

Can you rephrase?

The problems with endl arise from TRestStringOutput.h

[juanangp]

Does it arises from struct endl_t on TRestStringOutput.h? I think we should fix it in this PR...

[lobis]

I am not sure if I would complicate more this PR as its already big enough, perhaps @jgalan has a opinion. I agree that this should be fix, but perhaps on another PR if its not an easy fix. So far this doesn't seem to cause any problem, but it may have been unnoticed.

[jgalan]

TRestStringOutput.h was designed by @nkx, probably he has a better opinion than myself.

On the other hand, you need to create a new branch lobis-remove-namespace-std-from-headers at rawlib to fix compilation issues at this PR.

It would be great that a push to rawlib lobis-remove-namespace-std-from-headers would trigger the pipeline at this PR.

[lobis]

On the other hand, you need to create a new branch lobis-remove-namespace-std-from-headers at rawlib to fix compilation issues at this PR.

But this branch already exists, I don't understand the error. https://github.com/rest-for-physics/rawlib/tree/lobis-remove-namespace-std-from-headers

[nkx111]

The problems with endl arise from TRestStringOutput.h

What is the problem? In principle it is OK to remove using namespace std, because once we do that, we need to state std::end, which induces less ambiguity to the compiler. There will be less problems for my endl_t

[lobis]

The problems with endl arise from TRestStringOutput.h

What is the problem? In principle it is OK to remove using namespace std, because once we do that, we need to state std::end, which induces less ambiguity to the compiler. There will be less problems for my endl_t

If I remember correctly one of the problems is that you need to use std::endl (explicitly, event when using namespace std is declared) when using std::cout or it will throw an error. Also you need to use endl (without std::) when using metadata << and such or it will throw an error. I think it shouldn't behave like this.

[lobis]

I think this PR can be merged and causes no issues. It would be nice to fix the endl thing but I think it doesn't cause problems, its just annoying.

Since this is a big PR I think it should be merged as soon as possible (same for all linked PRs for libraries) before I need to fix too many conflicts. Let me know if you want something changed @jgalan @juanangp @nkx111

[lobis]

can we approve this merge request @jgalan @rest-for-physics/core_dev ? It doesn't pass the pipeline because submodules need to be updated, but it should be OK. what shall we do?

Also I did a very small change to the new curl in 45ca1db, I hope its ok. I think it would be nice to have a unit test in place for new additions such as this one, in order to promote testing.

[jgalan]

Fixed the issue with the process validation, now it seems the problem is somewhere else during compilation.

[lobis]

Fixed the issue with the process validation, now it seems the problem is somewhere else during compilation.

@jgalan the problem is due to restG4, its not using the master branch for the compilation

[jgalan]

In principle it says it is pulling master

packages/restG4 --> Pulling branch : master  [�[92m OK �[0m] (cc7486c)

Why don't you create a lobis-remove-namespace-std-from-headers at restG4 to try out?

[jgalan]

You could also try at the pipeline something like:

cd source/packages/restG4
git log

[lobis]

In principle it says it is pulling master

packages/restG4 --> Pulling branch : master  [�[92m OK �[0m] (cc7486c)

Why don't you create a lobis-remove-namespace-std-from-headers at restG4 to try out?

but the correct commit is [49d0345](https://github.com/rest-for-physics/restG4/commit/49d0345b2705d8e884d4cf8565688c303fb78977). I am sure its not using the correct version because of the logs, its calling different functions, for example 49d0345. I could fix it creating a dummy branch but we should find the root cause

[jgalan]

Yes, I guess creating the dummy branch will at least help to constrain and identify the problem.

[jgalan]

Up to now there was not problems, perhaps it happens when the last commit is a merge commit. I usually keep updated with master anytime, and then this problem did not appear before to me.

Solution might be modifying the way we pull submodules at pull-submodules.py

                         if latest == 1:
                             command = 'git ls-remote --heads ' + url + ' ' + frameworkBranchName + ' | wc -l'
                             branchExistsPcs = subprocess.run(command, shell=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE)

                             branchToPull = "master"
                             if( branchExistsPcs.stdout.decode("utf-8").rstrip("\n") != "0"):
                                 branchToPull = frameworkBranchName
                                 print( " --> Pulling branch : " + branchToPull + "  " , end='' )

                             p = subprocess.run('cd {}/{} && git pull --tags origin {}'.format(root, submodule, branchToPull), shell=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
                             if(debug):
                                 print (p.stdout.decode("utf-8"))
                                 print (p.stderr.decode("utf-8"))
                             errorOutput = p.stderr.decode("utf-8")
                             if errorOutput.find("failed") != -1 or errorOutput.find("error") != -1:
                                 print ("[\033[91m Failed \x1b[0m]")
                                 if(debug):
                                     print ("Message: ")
                                     print (errorOutput)
                                 continue

[lobis]

@jgalan @juanangp I updated the script with 764f628 and now the pipeline passes, its now pulling master branch when the named branch is not found. The only behavioural change is this commit, all other changes have no effect on usage of the script.

Please let me know if you think this can be merged now.