#142 Redefine default for branch ref:
Opened 4 years ago by vondruch. Modified 2 years ago

I originally proposed to change the reference defaults here [1], then I was asked to adjust the MBS [2] first. The later ticked was waived off based just on vague claims instead of proper discussion.

I am asking to have the proposal [1] accepted and worked on again, because apart all the arguments I already made, I realized that there is almost impossible to use development branches for module development. E.g. there is not possible to have devel branch rpms/foo and modules/foo and be able to send PR without changing the modulemd file (this would work only with forks).


Let me explain more why I am using "development branches for module development".
Because I want to have an only 1 commit by squashing some commits for each modules/foo and rpms/foo after the development.

For example, an actual case. I use below branch for modules/ruby,
When building modules/ruby. debugging rpms/ruby, I may have to add some commits for debug, empty commit to update the modules/ruby for new build.

  • modules/ruby 2.5 branch
  • rpms/ruby ruby-2.5 branch

If I use the development branch, finally I can do squash the commits to 1 commit when merging to modules/ruby "2.5" branch, and rpms/ruby "ruby-2.5" branch.
That is the reason.

The reason of I am using a development branch of the origin repository rather than the forked repository's branch is because "fedpkg module-bulid" does not support forked repository.
If "fedpkg module-bulid" supports forked repository, I think that using "forked repository" is better use case than "development branch" on the origin repository to keep clean the origin repository.

Metadata Update from @psabata:
- Issue tagged with: Meeting

4 years ago

This was discussed during today's meeting.

We're not fond of changing the defaults, however, we could introduce new, special named modulemd variables that would solve your issue as well.

For instance, something like ref: __REF_STREAM__ or ref: __REF_PLATFORM__ (for another use case that @ignatenkobrain brought up). Do you think that would work for you?

Metadata Update from @psabata:
- Issue untagged with: Meeting

4 years ago

We're not fond of changing the defaults

Would you mind to elaborate?

Do you think that would work for you?

It could work.

However, where I propose to follow convention over configuration, you are adding more configuration. Where I propose to simplify and remove things, you propose more stuff. Why should I study somewhere what __REF_STREAM__ or __REF_PLATFORM__ mean and how to use them if I can just omit them?

There are just a few who understand what modularity does, there is million pages of documentation where there is still not described enough basics and you propose to add another variables? This is really not sustainable.

Just FTR, for 8 years working on RHSCL, we never needed to specify branch anywhere. It just followed "the same branch name" convention. I agree that specifying branch might have its advantages, but we should start with simple convention and later add branch overriding if needed.

I should note that my proposal is perfectly backward compatible. Introducing new variables such as __REF_STREAM__ or __REF_PLATFORM__ means compatibility issues. There will be old versions of libraries, which won't be able to cope with them.

I should note that my proposal is perfectly backward compatible. Introducing new variables such as REF_STREAM or REF_PLATFORM means compatibility issues. There will be old versions of libraries, which won't be able to cope with them.

Your proposal is not backwards-compatible. An unspecified ref is defined in the specification as being exactly the same as ref: master and this is likely encoded in multiple places. Changing this requires a bump to the mdversion, which would make it incompatible with the libmodulemd shipped in EL 7 and EL 8.

However, because ref accepts an arbitrary string, we can add some reserved words to use as variables. Earlier versions of the libraries will interpret them just as the strings (in a practical sense, it will treat them as git branches/tags with that name). So it will remain compatible with the older libraries.

I should note that my proposal is perfectly backward compatible. Introducing new variables such as REF_STREAM or REF_PLATFORM means compatibility issues. There will be old versions of libraries, which won't be able to cope with them.

Your proposal is not backwards-compatible. An unspecified ref is defined in the specification as being exactly the same as ref: master and this is likely encoded in multiple places.

Yes, right, from theoretical POV, you are right. But from practical POV you are wrong, because there is not possible any collision.

If refs in modulemd do not specify branches, the master is used and it will continue to be used. If there is already specified branch, the specified branch keeps to be used. There does not exist the case which could fail if you add the "follow the modulemd branch" convention.

Changing this requires a bump to the mdversion, which would make it incompatible with the libmodulemd shipped in EL 7 and EL 8.

Based on my claims in previous paragraph, the bump is required just theoretically. Not practically.

I should note that my proposal is perfectly backward compatible. Introducing new variables such as REF_STREAM or REF_PLATFORM means compatibility issues. There will be old versions of libraries, which won't be able to cope with them.

Your proposal is not backwards-compatible. An unspecified ref is defined in the specification as being exactly the same as ref: master and this is likely encoded in multiple places.

Yes, right, from theoretical POV, you are right. But from practical POV you are wrong, because there is not possible any collision.
If refs in modulemd do not specify branches, the master is used and it will continue to be used. If there is already specified branch, the specified branch keeps to be used. There does not exist the case which could fail if you add the "follow the modulemd branch" convention.

Uh, how do you figure? The whole point of this request was to change the meaning of unspecified ref from meaning ref: master to meaning ref: <branch_name>. That is very much a change in behavior.

The whole point of this request was to change the meaning of unspecified ref from meaning ref: master to meaning ref: <branch_name>. That is very much a change in behavior.

This is not what i proposed. I proposed to change meaning of unspecified ref: from meaning ref: master to meaning ref: <branch_name> and if such branch does not exists, fallback back to ref: master. May be the default could be expressed as ref: <branch_name> or master.

IOW my proposal keeps the current functionality, but gives precedence to a new way of interpretation of ref:, which is not used ATM and cannot collide with anything.

Metadata Update from @asamalik:
- Issue tagged with: needs-design

4 years ago

Please see https://github.com/fedora-modularity/libmodulemd/pull/501

Note that this does not make any particular implementation mandatory, it just loosens the specification for the packager v3 format such that the build system is permitted to make its own decision about what an unspecified ref means.

I think it's important to explain to the reporter that a default value of ref is only set by MBS. There's no other piece of documentation, specification or libraries (e.g. libmodulemd) in Modularity which could influence it. MBS is the only piece of build pipe line which knows a git branch the modulemd YAML file is checkout from. So if the reporter needs changing the default value, he needs to aim his focus on MBS project (https://pagure.io/fm-orchestrator/issue/). Now MBS undergoes a consolidation of setting the default branch value in https://pagure.io/fm-orchestrator/issue/1735 to move from master to main.

I was thinking about adding a new field to modulemd-packager-v3 format which would override the default ref value. One would be, for instance, able to specify ref: stream-%n-%s on top level and all undefined component refs would fall back to stream-NAME-STREAM value. But unfortunately without support from MBS side it would not work because MBS retrieves component refs before converting the format into a build-time modulemd-v2 format with libmodulemd library. I even wanted to support %p and %c formatting string for a platform stream and a context, but that is not possible for the very same reason.

Nonetheless even the top-level override would not be enough to cover branch scheme used in RHEL where components are located in branches whose name depends on a branch name of the modulemd YAML document. MBS would need to pass the branch name to libmodulemd. It does not do so. It passes a stream name. And branch name and stream name do not match in RHEL.

As you can see all paths lead to MBS. There is not much a modularity team can help you because MBS is not maintained by the modularity team. I can start a discussion on Fedora's devel list and based on the outcome we can reopen https://pagure.io/fm-orchestrator/issue/1226.

I think it's important to explain to the reporter that a default value of ref is only set by MBS. There's no other piece of documentation, specification or libraries (e.g. libmodulemd) in Modularity which could influence it.

There was such place when I opened this ticket and I think this PR was important step to unblock this. So no need to explain this to me.

Login to comment on this ticket.

Metadata