[x265] [X265] [Fix Patch] [Master, Release 3.5 branches] fix: avoids unnecessary lexicographic order checks on git changesets, corrects version reporting

[Srikanth Kurapati]

On Tue, Dec 22, 2020 at 7:00 PM Aruna Matheswaran <
aruna at multicorewareinc.com> wrote:

>
>
> On Tue, Dec 22, 2020 at 2:25 PM Srikanth Kurapati <
> srikanth.kurapati at multicorewareinc.com> wrote:
>
>>
>> From 123a406a4845ec228bd234693115e6ffb6bc423f Mon Sep 17 00:00:00 2001
>> From: Srikanth Kurapati <srikanth.kurapati at multicorewareinc.com>
>> Date: Wed, 16 Dec 2020 10:52:33 +0530
>> Subject: [PATCH] fix: avoids unnecessary lexicographic order checks on git
>>  changesets
>>
>> and correct variables used for git archive version reporting
>> ---
>>  source/cmake/Version.cmake | 28 ++++++++++------------------
>>  1 file changed, 10 insertions(+), 18 deletions(-)
>>
>> diff --git a/source/cmake/Version.cmake b/source/cmake/Version.cmake
>> index 37c759268..21730cf91 100644
>> --- a/source/cmake/Version.cmake
>> +++ b/source/cmake/Version.cmake
>> @@ -152,14 +152,14 @@ elseif(GIT_ARCHETYPE STREQUAL "1")
>>              message(WARNING "RELEASE CHANGESET INFO NOT PRESENT IN
>> VERSION FILE")
>>          endif()
>>          if(DEFINED git_repositorychangeset)
>> -           string(SUBSTRING "${git_repositorychangeset}" 0 9
>> X265_REPO_ID)
>> +            string(SUBSTRING "${git_repositorychangeset}" 0 9
>> X265_REPO_ID)
>>          else()
>> -           message(STATUS "X265 LATEST COMMIT TIP INFORMATION NOT
>> AVAILABLE")
>> +            message(STATUS "X265 LATEST COMMIT TIP INFORMATION NOT
>> AVAILABLE")
>>          endif()
>>          if(DEFINED git_releasetagdistance)
>> -           set(X265_TAG_DISTANCE ${git_releasetagdistance})
>> +            set(X265_TAG_DISTANCE ${git_releasetagdistance})
>>          else()
>> -           message(WARNING "COMMIT INFORMATION AFTER LATEST REVISION
>> UNAVAILABLE")
>> +            message(WARNING "COMMIT INFORMATION AFTER LATEST REVISION
>> UNAVAILABLE")
>>          endif()
>>      else()
>>          message(WARNING "X265 RELEASE VERSION LABEL MISSING:
>> ${X265_LATEST_TAG}")
>> @@ -168,25 +168,17 @@ endif()
>>
>>  # formatting based on positive or negative distance from tag
>>  if(X265_TAG_DISTANCE STREQUAL "0")
>> -    if(X265_REVISION_ID STREQUAL X265_REPO_ID)
>> -        set(X265_VERSION "${X265_LATEST_TAG}")
>> -    else()
>> -        message(WARNING "REPO AND RELEASE CHANGESETS NOT MATCHING")
>> -    endif()
>> +    set(X265_VERSION "${X265_LATEST_TAG}")
>>  elseif(X265_TAG_DISTANCE STRGREATER "0")
>> -    if(X265_REVISION_ID STRLESS X265_REPO_ID)
>> -        set(X265_VERSION
>> "${X265_LATEST_TAG}+${X265_TAG_DISTANCE}-${X265_REVISION_ID}")
>> +    if(GIT_ARCHETYPE STREQUAL "1")
>>
> [AM] Why aren't we handling HG here?
>
   [KS] In case of hg the entry "node: value" in the .hg_archival.txt file
always contains the tip at which the archive
          is created and the same is used to update X265_REVISION_ID.

> +        set(X265_VERSION
>> "${X265_LATEST_TAG}+${X265_TAG_DISTANCE}-${X265_REPO_ID}")
>>
>  [AM] Use X265_REVISION_ID to hold "repositoryChangeset" in both the cases
> ( i.e ARCHIVE type 1 and 2).
>
     [KS] done

>      else()
>> -        message(WARNING "ARCCHIVE TIP CHANGESET TO BE GREATER THAN
>> REVISION ID")
>> +        set(X265_VERSION
>> "${X265_LATEST_TAG}+${X265_TAG_DISTANCE}-${X265_REVISION_ID}")
>>      endif()
>>  elseif(X265_TAG_DISTANCE STRLESS "0")
>> -    if(X265_REVISION_ID STRGREATER X265_REPO_ID)
>> -       set(X265_VERSION
>> "${X265_LATEST_TAG}${X265_TAG_DISTANCE}+${X265_REPO_ID}")
>> -    else()
>> -        message(WARNING "REVISION ID EXPECTED TO BE LARGER THAN ARCHIVE
>> TIP CHANGESET")
>> -    endif()
>> +    set(X265_VERSION
>> "${X265_LATEST_TAG}${X265_TAG_DISTANCE}+${X265_REPO_ID}")
>>
> [AM] With the clean up mentioned above, this section shall be eliminated.
>
    [KS] removed this section fully.

>
> [AM] Treating  GIT_ARCHETYPE as a flag will improve code readability.
>
   [KS] same holds for HG_ARCHETYPE too. Addressed in both the cases, also
add necessary comments for code readability, output messages for user
understanding.

>  else()
>> -    message(ERROR "Inappropriate set of version information")
>> +    message(ERROR "Inappropriate version information")
>>
> [AM] This else() will never take a hit. Remove this.
>
   [KS] Yes done.

>  endif()
>>
>>  #will always be printed in its entirety based on version file
>> configuration to avail revision monitoring by repo owners
>> --
>> 2.20.1.windows.1
>>
>> --
>> *With Regards,*
>> *Srikanth Kurapati.*
>> _______________________________________________
>> x265-devel mailing list
>> x265-devel at videolan.org
>> https://mailman.videolan.org/listinfo/x265-devel
>>
>
>
> --
> Regards,
> *Aruna Matheswaran,*
> Video Codec Engineer,
> Media & AI analytics BU,
>
>
>
> _______________________________________________
> x265-devel mailing list
> x265-devel at videolan.org
> https://mailman.videolan.org/listinfo/x265-devel
>


-- 
*With Regards,*
*Srikanth Kurapati.*
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/x265-devel/attachments/20201223/e971f415/attachment.html>