[PATCH] build: Add optional RTEMS_VERSION_VC_KEY

[Chris Johns]

On 31/7/2023 5:14 pm, Sebastian Huber wrote:
> On 29.07.23 08:30, Chris Johns wrote:
>>> I think this is not really related to the new
>>> RTEMS_VERSION_VC_KEY/RTEMS_VERSION_CONTROL_KEY option. The purpose of this
>>> option is to give users the ability to define their own version control key.
>>
>> I think we are broadly aligned however there are different labels for some the
>> pieces [1]. I have reviewed the existing code and then this change and it
>> seems the default is an empty string a user can altered via a config.ini file.
>> Is this correct?
> 
> In v2 of the patch, the default is "git" indicating that the version control key
> should be obtained using Git.

What version control keys are supported?

>>
>> Is this per BSP or global?
> 
> This is per BSP. All the configuration options are per BSP. In your config.ini
> you can set options for all BSPs via the [DEFAULT] section.

Per BSP does not make sense. Having a means for a user to inject anything at
build time is confusing to me because it can be different for the same set of
sources.

>> For me the sources are the controlled item the calls rtems_version() and
>> rtems_version_control_key() should report and not what a user can configure.
> 
> We don't control the version control system of the user, so it should be user
> configurable.

The configuration file controls a build and this setting changes the meaning of
the configuration data being built. It does not align.

>> Why not just look for a file called VERSION and then a key=value pair that is:
>>
>> RTEMS_VERSION_VC_KEY=foo-bar
> 
> Why not just use what is normally used by the build system? Supporting the
> VERSION file needs extra code for only one purpose.

Yes releasing RTEMS. At the moment a get clone of a release tag will show a file
does not match. In a formal sense that would require a special consideration
that would need to be documented and applied.

> I think naming this option RTEMS_VERSION_CONTROL_KEY makes more sense, since the
> corresponding RTEMS API directive is rtems_version_control_key(). Is VC a well
> known abbreviation?

The name in the API is what it is and I am not suggesting it be changed. Maybe
it could have been more general to serve a wider purpose than just the VC but it
is there now so that time has passed.

>>
>> And document deployment users add this file and then configuration control the
>> released sources and this file?
> 
> The config.ini are not included in the sources shipped with RTEMS and they can
> be version controlled by the user.

That is the central point I am attempting to make. Version data should be the
version of the sources shipped and not the build.

A build label is different and would clearly state the purpose and if I see a
bug report I can ignore it and concentrate on the version data.

Chris