Support crypto config usage in depends.py

[gabor-mezei-arm]

To support the depnds.py update:

• Move config file backup functionality to config_common.py
• Capture output of C file building process to support consistency check

Related to Mbed-TLS/mbedtls#9292

[minosgalanakis]

We are using instance attributes to control the execution flow of a metaclass? While it may work now, all it wold take to start doing unpredictable thigns is someone to set a default value anywhere in the inheritable chain.

CombinedConfig._backupname ----
                              |  ----> MbedTLSConfigFile._backupname  ----> ConfigFile._backupname
                              | 
                              L ----> CryptoConfigFilee._backupname  -----> ConfigFile._._backupname  

Can we attack the logic to the actual file being present on the directory instead?

P.S: Why is ConfigFile a metaclass?

[gabor-mezei-arm]

We are using instance attributes to control the execution flow of a metaclass? While it may work now, all it wold take to start doing unpredictable thigns is someone to set a default value anywhere in the inheritable chain.

_backupname is a simple private variable. Each instance has its own and it is not shared. Do not cause any problem.

Can we attack the logic to the actual file being present on the directory instead?

If a file with the desired file name is present then it will not create a copy. That will be the backup. This logic will preserve the original if more then one process is modifying the config.

P.S: Why is ConfigFile a metaclass?

It is abstract to force to create different derived classes for the different kind of config files to easily differentiate them.

[minosgalanakis]

_backupname is a simple private variable.

Unfortunately that is not the case. What we care about is the lifecyle of the variable and weather we are creating it on runtime, before instantiation or during class constcution, at startup.

In the ConfigFile

class ConfigFile(metaclass=ABCMeta):
    """Representation of a configuration file."""
       ...
        self._backupname = None
        self._own_backup = False

Because that is a metaclass, that ConfigFile._backupname is not just a class variable which is created after the __new__ method is called, but it is created as an attribute to __type__ and passed to the `__new__¬ consturctor to make the class. All classes that inherit it will have it as a class variable.

At any rate that code will work, but is very prone to shoot us in the foot since we do not contorl how the interpret will dynamically generate the classes. We can either add a comment to never change the default line, or only create _hasbackup when the backup() method is called which is guaranteed to happen during runtime and after instantation. We can use hasattr() method to check if that is present.

It is abstract to force to create different derived classes for the different kind of config files to easily differentiate them.

You mean that we need them to be dynamically typed and not created using instantiation? Wouldn't simple inheritace work, with the cost of having to call init, when we actually have a config file?

Either way I won't block this pr for it, but I would like to point out that we are introducing complexity that brings more risk and technical debt than it's benefits.

[gabor-mezei-arm]

Because that is a metaclass, that ConfigFile._backupname is not just a class variable which is created after the __new__ method is called, but it is created as an attribute to __type__ and passed to the `__new__¬ consturctor to make the class. All classes that inherit it will have it as a class variable.

I think you are confused with the number of underscores. _ (one underscore) means it is a private variable (only naming convention, does nothing), __ (two underscores) uses name mangling and does what you mentioned.

Can be checked here.

[ronald-cron-arm]

Thus regarding class ConfigFile(metaclass=ABCMeta): after some offline discussion with @gabor-mezei-arm my understanding is that the intent is to declare the ConfigFile as an abstract class to prevent it for being instantiated.
Looking at some resources on the web like https://docs.python.org/3/library/abc.html this seems a reasonable way to do it.

Otherwise because ConfigFile is declared as class ConfigFile(metaclass=ABCMeta): does not seem to mean that ConfigFile is a metaclass. It seems to just mean that the object created to hold the definition of the ConfigFile class is of type ABCMeta and not type. Then ConfigFile class itself could be a metaclass but not necessarily and it does not seem to be with the signature of its init function as we have in config_common.py.

If I run the following script

#!/usr/bin/env python3                                                          
                                                                                
from abc import ABCMeta                                                      
                                                                                
class ConfigFile0:                                                              
    def __init__(self):                                                         
        self._backupname = None                                                 
                                                                                
class ConfigFile1(metaclass=ABCMeta):                                           
    def __init__(self):                                                         
        self._backupname = None                                                 
                                                                                
print(type(ConfigFile0))                                                        
print(type(ConfigFile1))                                                        
                                                                                
class ConfigFile2(metaclass=ConfigFile1):                                       
    pass

I get:

<class 'type'>
<class 'abc.ABCMeta'>
Traceback (most recent call last):
  File "/home/roncro01/./test1.py", line 16, in <module>
    class ConfigFile2(metaclass=ConfigFile1):
TypeError: ConfigFile1.__init__() takes 1 positional argument but 4 were given

The init function of ConfigFile1 does not have the proper signature for a metaclass. It seems it should be something like: def __init__(cls, name, bases, dct):

[ronald-cron-arm]

Just a type, otherwise, this looks good to me.

[ronald-cron-arm]

Suggested change

	"""Excetion to represent an error during the compilation."""
	"""Exception to represent an error during the compilation."""

[ronald-cron-arm]

Thus regarding class ConfigFile(metaclass=ABCMeta): after some offline discussion with @gabor-mezei-arm my understanding is that the intent is to declare the ConfigFile as an abstract class to prevent it for being instantiated.
Looking at some resources on the web like https://docs.python.org/3/library/abc.html this seems a reasonable way to do it.

Otherwise because ConfigFile is declared as class ConfigFile(metaclass=ABCMeta): does not seem to mean that ConfigFile is a metaclass. It seems to just mean that the object created to hold the definition of the ConfigFile class is of type ABCMeta and not type. Then ConfigFile class itself could be a metaclass but not necessarily and it does not seem to be with the signature of its init function as we have in config_common.py.

If I run the following script

#!/usr/bin/env python3                                                          
                                                                                
from abc import ABCMeta                                                      
                                                                                
class ConfigFile0:                                                              
    def __init__(self):                                                         
        self._backupname = None                                                 
                                                                                
class ConfigFile1(metaclass=ABCMeta):                                           
    def __init__(self):                                                         
        self._backupname = None                                                 
                                                                                
print(type(ConfigFile0))                                                        
print(type(ConfigFile1))                                                        
                                                                                
class ConfigFile2(metaclass=ConfigFile1):                                       
    pass

I get:

<class 'type'>
<class 'abc.ABCMeta'>
Traceback (most recent call last):
  File "/home/roncro01/./test1.py", line 16, in <module>
    class ConfigFile2(metaclass=ConfigFile1):
TypeError: ConfigFile1.__init__() takes 1 positional argument but 4 were given

The init function of ConfigFile1 does not have the proper signature for a metaclass. It seems it should be something like: def __init__(cls, name, bases, dct):

[ronald-cron-arm]

LGTM

[minosgalanakis]

Looks good

[ronald-cron-arm]

@gabor-mezei-arm we need a 3.6 PR that just update the framework pointer to the head of this PR to validate it for 3.6. Then we can merge it.

[ronald-cron-arm]

Validated by #9292 (dev) and #9709 (3.6) CI.