On 1/4/19 7:12 AM, Penney, Don wrote:

From a patching perspective, which is why TIS_PATCH_VER was introduced originally, it can be reset to 0 when the source package is upversioned. But I see Scott's point from his review comment about indicating a revision from source, and Chris's below.

Setting it to 1 to show modification from original source seems reasonable to me. Given that it will get incremented and veer from the patch count, I don't see a lot of benefit to needing to count the patches to determine an initial version. But if we're going that route, I'd vote for b - count the number of patch files total.

I am not sure I agree with any of this, first off, just the fact that we have an SRPM and the TIS_PACTH_VER indicates that it's been patched, I really don't see the value in having the patch count indicated as a "Version" item. It makes more sense to start from 0 (option a) and that way we can track each subsequent change to that package with an increment. This issue did not come up at all in past updates, I am not sure why it's becoming an issue now. See below for additional comments

-----Original Message----- From: Friesen, Chris Sent: Friday, January 04, 2019 9:46 AM To: An, Ran1; Lin, Shuicheng; Penney, Don; Saul Wold; Little, Scott; Church, Robert; Bailey, Henry Albert (Al) Cc: starlingx-discuss@lists.starlingx.io; Chen, Haochuan Z Subject: Re: [Starlingx-discuss]discuss about initial value of TIS_PATCH_VER when upgrade packages

When we customize an upstream package for the first time, TIS_PATCH_VER gets set to 1, then generally gets incremented on each subsequent change.  Thus, prior to package upgrade TIS_PATCH_VER reflects the number of changes that were made to the upstream package.  This can be used to tell at a glance how customized a given package is.

When upgrading, it's possible that some customizations are no longer applicable, while others are.  Thus, I think options "a" and "e" don't make sense as they remove the "how customized is this package" meaning.

As mentioned above, just having that additional tis.<TIS_PATCH_VER> in the file name indicates that it's been modified.

Of the options below, I think option "c" is probably the best since for an upgrade we might create a single meta-patch to add all the source patches.

And what happens when a modification is needed to the Specfile or patch with out increasing the actual number of patches, now the value of TIS_PATCH_VER increments and no longer matches the patch count. Therefore, a version should be incremental from 0. Sau!

I think the most accurate value would probably be "number of source patches" plus "number of meta patches that don't add/remove source patches".  But we probably don't really need that level of accuracy.

Chris

On 1/4/2019 2:28 AM, An, Ran1 wrote:

...

Hi all I'm sending this to discuss about the rule of initial value of TIS_PATCH_VER when srpm package is upgraded. "TIS_PATCH_VER" is a counter to indicate change within a major version of the package, on which we put patches.

When I upgraded srpms(related to CentOS) from CentOS 7.5 to 7.6, there are different voices about the initial value of TIS_PATCH_VER(comments on [1][2][3][4]): a). reset it to 0 b). reset to the number of STX patches remaining (source patches and meta_patches together) c). reset to the number of STX patches remaining (source patches only) d). reset to the number of STX patches remaining (meta patches only) e). case by case, better do not reset.

It is not a technical issue, but we will face it each time we upgrade packages, so which would you like to choose?

[1] https://review.openstack.org/#/c/627760/ [2] https://review.openstack.org/#/c/627750/ [3] https://review.openstack.org/#/c/627156/ [4] https://review.openstack.org/#/c/627770/

Thanks Ran

On 1/7/19 7:28 AM, Scott Little wrote:

I disagree.   Our experience in the past, is that putting a tis.0 on a package raises questions from both customers and designers.  Why are you compiling this at all if you aren't changing it?I would have thought that the tis.<x> extension would be enough to indicate this package had patches.

I also think we should really be switching to stx.0, but that's a different discussion I would guess.

A little digging, and some wasted cycles, and the answer is.  "Oh, we are changing it. we still have 3 patches against it. sorry for the confusion."

Now as you point out.  We might remove a patch in a non-rebase context. In this case we are compelled to increment, rather than decrement, TIS_PACTH_VER.  In this case we have to live with the misleadingly high number until the next rebase.  That's ok.  No one has complained about that.

I guess I am about the consistency of the meaning of tis.<x> when it increments, such that starting at 0 and later incrementing means change occurs vs starting at N want meaning a patch count and later incrementing and not really having a meaning any more, my OCD kind of kicks in.

I should have been flagging this in earlier code reviews.  I wasn't.  My error.  Had bigger fish to fry in the early months of going open source.

As I said, I had never heard this until now, I understand your busy, but we did the whole 7.5 update without hearing about.

If the community wants to overrule, that's fine.  I'm just trying to share my hard won experience as 'the rebase guy' for 4 years prior to open sourcing.

Do we need a proper Specification for the meaning of the package information, this is where we can change the tis/TIS to stx/STX! Sau!

Scott

On 2019-01-04 4:52 p.m., Saul Wold wrote:

...

I am not sure I agree with any of this, first off, just the fact that we have an SRPM and the TIS_PACTH_VER indicates that it's been patched, I really don't see the value in having the patch count indicated as a "Version" item.

It makes more sense to start from 0 (option a) and that way we can track each subsequent change to that package with an increment.

This issue did not come up at all in past updates, I am not sure why it's becoming an issue now.

See below for additional comments

I agree that we'd eventually want to switch to "stx" instead of "tis". The way that we did it previously was to consistently have the meaning of the ".x" be "the number of changes made to the upstream package".  So the first time you make a change it'd be ".1", then you make another change and it'd be ".2", and then if you upgrade to a newer base package but keep both changes it'd have a new upstream base version but still be ".2" for the version suffix. For what it's worth, CentOS and Debian do things a bit differently.  When they move to a new upstream version of the package they switch back to "-1" regardless of the number of patches .  So you'd have something like 0.14.0-1, then 0.14.0-2, then 0.15.2-1.  OpenSUSE has a more complicated suffix like "-5.3.1", I'm not sure what their rules for updating it are. Given the above, I could see a rationale for reducing confusion by aligning with CentOS and switching back to ".1" when bumping  upstream versions.  But I still think there is value in the previous mechanism as it gives a general idea of how much a given package differs from upstream. Chris On 1/7/2019 5:48 PM, Saul Wold wrote:

I also think we should really be switching to stx.0, but that's a different discussion I would guess.

<snip>

I guess I am about the consistency of the meaning of tis.<x> when it increments, such that starting at 0 and later incrementing means change occurs vs starting at N want meaning a patch count and later incrementing and not really having a meaning any more, my OCD kind of kicks in.

We had a quick discussion here at the F2F with some WRS folks on the phone. We came to an agreement that the value of TIS_PATCH_VER would be reset to 1 when a rebase occurs and incremented by 1 each additional change that is made to the specfile. As part of the future "re-branding" plan we will rename TIS_PATCH_VER to STX_PATCH_VER along with the %_tis_dist/%_tis_patch_ver variables. For the current rebase in flight, what's merged is merged and what's pending should be updated to match this new understanding. Thanks for your support! Sau! On 1/7/19 7:10 AM, Scott Little wrote:

100% agree.

Scott

On 2019-01-04 9:46 a.m., Chris Friesen wrote:

...

When we customize an upstream package for the first time, TIS_PATCH_VER gets set to 1, then generally gets incremented on each subsequent change.  Thus, prior to package upgrade TIS_PATCH_VER reflects the number of changes that were made to the upstream package.  This can be used to tell at a glance how customized a given package is.

When upgrading, it's possible that some customizations are no longer applicable, while others are.  Thus, I think options "a" and "e" don't make sense as they remove the "how customized is this package" meaning.

Of the options below, I think option "c" is probably the best since for an upgrade we might create a single meta-patch to add all the source patches.

I think the most accurate value would probably be "number of source patches" plus "number of meta patches that don't add/remove source patches".  But we probably don't really need that level of accuracy.

Chris

On 1/4/2019 2:28 AM, An, Ran1 wrote:

...

Hi all    I'm sending this to discuss about the rule of initial value of TIS_PATCH_VER when srpm package is upgraded. "TIS_PATCH_VER" is a counter to indicate change within a major version of the package, on which we put patches.    When I upgraded srpms(related to CentOS) from CentOS 7.5 to 7.6, there are different voices about the initial value of TIS_PATCH_VER(comments on [1][2][3][4]):      a). reset it to 0      b). reset to the number of STX patches remaining (source patches and meta_patches together)      c). reset to the number of STX patches remaining (source patches only)      d). reset to the number of STX patches remaining (meta patches only)      e). case by case, better do not reset.

It is not a technical issue, but we will face it each time we upgrade packages, so which would you like to choose?

[1] https://review.openstack.org/#/c/627760/ [2] https://review.openstack.org/#/c/627750/ [3] https://review.openstack.org/#/c/627156/ [4] https://review.openstack.org/#/c/627770/

Thanks Ran