Re: [meta-intel] [PATCH] IGT: update 1.14 -> 1.15

[Jianxun Zhang]

On Sep 11, 2016, at 1:17 PM, Paul Eggleton  
wrote:
> 
> Hi Jianxin,
> 
> On Fri, 09 Sep 2016 10:50:50 Jianxun Zhang wrote:
>>> On Sep 9, 2016, at 7:34 AM, Saul Wold  wrote:
>>> On Thu, 2016-09-08 at 17:02 -0700, Jianxun Zhang wrote:
 This change update intel-gpu-tools to 1.15 as a sync-up with
 Intel graphic stack 2016 Q2 release.
 
 This change explicitly sets ${PV} in recipe and renames it
 after package name only. By doing so we don't need to enforce
 a policy to rename recipe every time we do update. Patch
 speaks itself.
>>> 
>>> This is wrong!
>>> 
>>> The whole point of naming the recipe as ${PN}_${PV} is to remove the
>>> need to update PV in the recipe.
>>> 
>>> Yes checksums need to be updated on a regular basis, there are tools
>>> like devtool that can assist with this.
>>> 
>>> The OpenEmbedded standard is for $PN_$PV.bb filename this also assists
>>> in understand quickly what version a particular recipe is.  There are
>>> also limited cases where 2 different $PVs are needed for some reasons.
>>> 
>>> Please resubmit this as a normal update with $PV contained in the file
>>> name.
>>> 
>>> Please see: https://wiki.yoctoproject.org/wiki/Best_Known_Methods_(BKMs
>>> )_for_Package_Updating
>> 
>> Saul,
>> I think the drawback of current procedure is it causes git “resets" history
>> on recipe since new recipe when a renamed or git mv-ed recipe also have too
>> much modifications. I don’t think git really tracks moving or renaming:
> 
> It doesn't reset history. You're right that it doesn't track renames - that's 
> because rename detection is done whenever git looks at a change, not when the 
> change is applied. If you want to see the full history including renames, use 
> the --follow option.
Paul,
I just come back from a vacation and run "git log --follow new_renamed_file" in 
the test project created with procedure in my previous reply. It doesn’t show 
history before renaming the file in that case.
The procedure simulates a routine update (renaming & modifying recipe in a 
single commit) but won’t always give the expected result in git history. I 
think the first half of your comment could be the answer. git doesn’t   track 
rename, so --follow should not have more magic in this case.

> 
> Saul is correct, use of PV in the recipe filename is standard OE practice and 
> we don't want to be deviating from that - besides which I'm not sure we have 
> much to gain by doing so.

If you defined this as a standard in OE universe, no more argument is needed. I 
just want to point out any inconsistent outcome from the standard practice  
which may be out of expectation sometimes, assuming my test is valid.

Last time I followed the policy to use git mv and submit a patch to update a 
short recipe in meta-intel, we had same confusion when Saul didn’t see renaming 
info in patch in review. (I can’t remember which recipe however).

I will submit a V2 back to normal.

Thanks lot!

> 
> Cheers,
> Paul
> 
> -- 
> 
> Paul Eggleton
> Intel Open Source Technology Centre

-- 
___
meta-intel mailing list
meta-intel@yoctoproject.org
https://lists.yoctoproject.org/listinfo/meta-intel

Re: [meta-intel] [PATCH] IGT: update 1.14 -> 1.15

[Saul Wold]

On Mon, 2016-09-12 at 08:17 +1200, Paul Eggleton wrote:
> Hi Jianxin,
> 
> On Fri, 09 Sep 2016 10:50:50 Jianxun Zhang wrote:
> > 
> > > 
> > > On Sep 9, 2016, at 7:34 AM, Saul Wold 
> > > wrote:
> > > On Thu, 2016-09-08 at 17:02 -0700, Jianxun Zhang wrote:
> > > > 
> > > > This change update intel-gpu-tools to 1.15 as a sync-up with
> > > > Intel graphic stack 2016 Q2 release.
> > > > 
> > > > This change explicitly sets ${PV} in recipe and renames it
> > > > after package name only. By doing so we don't need to enforce
> > > > a policy to rename recipe every time we do update. Patch
> > > > speaks itself.
> > > 
> > > This is wrong!
> > > 
> > > The whole point of naming the recipe as ${PN}_${PV} is to remove
> > > the
> > > need to update PV in the recipe.
> > > 
> > > Yes checksums need to be updated on a regular basis, there are
> > > tools
> > > like devtool that can assist with this.
> > > 
> > > The OpenEmbedded standard is for $PN_$PV.bb filename this also
> > > assists
> > > in understand quickly what version a particular recipe is.  There
> > > are
> > > also limited cases where 2 different $PVs are needed for some
> > > reasons.
> > > 
> > > Please resubmit this as a normal update with $PV contained in the
> > > file
> > > name.
> > > 
> > > Please see: https://wiki.yoctoproject.org/wiki/Best_Known_Methods
> > > _(BKMs
> > > )_for_Package_Updating
> > 
> > Saul,
> > I think the drawback of current procedure is it causes git “resets"
> > history
> > on recipe since new recipe when a renamed or git mv-ed recipe also
> > have too
> > much modifications. I don’t think git really tracks moving or
> > renaming:
> 
> It doesn't reset history. You're right that it doesn't track renames
> - that's 
> because rename detection is done whenever git looks at a change, not
> when the 
> change is applied. If you want to see the full history including
> renames, use 
> the --follow option.
> 
> Saul is correct, use of PV in the recipe filename is standard OE
> practice and 
> we don't want to be deviating from that - besides which I'm not sure
> we have 
> much to gain by doing so.
> 
Just to continue to clarify further:
1) The version adds uniqueness when there are multiple version
2) It's human readable for parsing by the eyes
3) There are some performance benefits to scripts and tools since they
do not have to open the file to parse out the PV 

Sau!

> Cheers,
> Paul
> 
-- 
___
meta-intel mailing list
meta-intel@yoctoproject.org
https://lists.yoctoproject.org/listinfo/meta-intel

Re: [meta-intel] [PATCH] IGT: update 1.14 -> 1.15

[Paul Eggleton]

Hi Jianxin,

On Fri, 09 Sep 2016 10:50:50 Jianxun Zhang wrote:
> > On Sep 9, 2016, at 7:34 AM, Saul Wold  wrote:
> > On Thu, 2016-09-08 at 17:02 -0700, Jianxun Zhang wrote:
> >> This change update intel-gpu-tools to 1.15 as a sync-up with
> >> Intel graphic stack 2016 Q2 release.
> >> 
> >> This change explicitly sets ${PV} in recipe and renames it
> >> after package name only. By doing so we don't need to enforce
> >> a policy to rename recipe every time we do update. Patch
> >> speaks itself.
> > 
> > This is wrong!
> > 
> > The whole point of naming the recipe as ${PN}_${PV} is to remove the
> > need to update PV in the recipe.
> > 
> > Yes checksums need to be updated on a regular basis, there are tools
> > like devtool that can assist with this.
> > 
> > The OpenEmbedded standard is for $PN_$PV.bb filename this also assists
> > in understand quickly what version a particular recipe is.  There are
> > also limited cases where 2 different $PVs are needed for some reasons.
> > 
> > Please resubmit this as a normal update with $PV contained in the file
> > name.
> > 
> > Please see: https://wiki.yoctoproject.org/wiki/Best_Known_Methods_(BKMs
> > )_for_Package_Updating
> 
> Saul,
> I think the drawback of current procedure is it causes git “resets" history
> on recipe since new recipe when a renamed or git mv-ed recipe also have too
> much modifications. I don’t think git really tracks moving or renaming:

It doesn't reset history. You're right that it doesn't track renames - that's 
because rename detection is done whenever git looks at a change, not when the 
change is applied. If you want to see the full history including renames, use 
the --follow option.

Saul is correct, use of PV in the recipe filename is standard OE practice and 
we don't want to be deviating from that - besides which I'm not sure we have 
much to gain by doing so.

Cheers,
Paul

-- 

Paul Eggleton
Intel Open Source Technology Centre
-- 
___
meta-intel mailing list
meta-intel@yoctoproject.org
https://lists.yoctoproject.org/listinfo/meta-intel

Re: [meta-intel] [PATCH] IGT: update 1.14 -> 1.15

[Jianxun Zhang]

> On Sep 9, 2016, at 7:34 AM, Saul Wold  wrote:
> 
> On Thu, 2016-09-08 at 17:02 -0700, Jianxun Zhang wrote:
>> 
>> This change update intel-gpu-tools to 1.15 as a sync-up with
>> Intel graphic stack 2016 Q2 release.
>> 
>> This change explicitly sets ${PV} in recipe and renames it
>> after package name only. By doing so we don't need to enforce
>> a policy to rename recipe every time we do update. Patch
>> speaks itself.
>> 
> This is wrong!  
> 
> The whole point of naming the recipe as ${PN}_${PV} is to remove the
> need to update PV in the recipe.
> 
> Yes checksums need to be updated on a regular basis, there are tools
> like devtool that can assist with this.
> 
> The OpenEmbedded standard is for $PN_$PV.bb filename this also assists
> in understand quickly what version a particular recipe is.  There are
> also limited cases where 2 different $PVs are needed for some reasons.
> 
> Please resubmit this as a normal update with $PV contained in the file
> name.
> 
> Please see: https://wiki.yoctoproject.org/wiki/Best_Known_Methods_(BKMs
> )_for_Package_Updating
Saul,
I think the drawback of current procedure is it causes git “resets" history on 
recipe since new recipe when a renamed or git mv-ed recipe also have too much 
modifications. I don’t think git really tracks moving or renaming:

This is what I did with wiki procedure:

mkdir gmtest
cd gmtest
git init
echo "file 1" > file
git add .
#/ now we have old recipe:
git commit -s -m "commit 1”
# lets update it, following wiki:
git mv file file_new
git status
# modify renamed new recipe:
echo "file 2" > file_new
git status
git diff
git add .
# git won’t show renaming info because of similarity
git commit -s -m "commit 2”
# git wont’ show commit #1 for old recipe - chopped history:
git log file_new
# git won’t have renaming info in final patch:
git format-patch -M -1
# and you can’t run this with tip at commit #2 for the old
git log file

Thanks

> 
> Thanks 
>Sau!
> 
> 
>> 
>> ${PV} and checksums shall be updated together. When either
>> checksums or ${PV} is forgotten to update, developers will
>> get a build error as a heads-up. This is because graphic base
>> in OE fetches source with ${PV}.
>> 
>> Signed-off-by: Jianxun Zhang 
>> ---
>> Rough test is done by cherry-picking some IGT tools and tests.
>> I still need to hack piglit to run IGT test as an authentic test
>> approach, but I will address this with another patch to OE.
>> 
>>  .../intel-gpu-tools/{intel-gpu-tools_1.14.bb => intel-gpu-tools.bb}
>>> 
>>> 6 --
>>  1 file changed, 4 insertions(+), 2 deletions(-)
>>  rename common/recipes-graphics/intel-gpu-tools/{intel-gpu-
>> tools_1.14.bb => intel-gpu-tools.bb} (88%)
>> 
>> diff --git a/common/recipes-graphics/intel-gpu-tools/intel-gpu-
>> tools_1.14.bb b/common/recipes-graphics/intel-gpu-tools/intel-gpu-
>> tools.bb
>> similarity index 88%
>> rename from common/recipes-graphics/intel-gpu-tools/intel-gpu-
>> tools_1.14.bb
>> rename to common/recipes-graphics/intel-gpu-tools/intel-gpu-tools.bb
>> index c20ce31..64cc612 100644
>> --- a/common/recipes-graphics/intel-gpu-tools/intel-gpu-tools_1.14.bb
>> +++ b/common/recipes-graphics/intel-gpu-tools/intel-gpu-tools.bb
>> @@ -15,8 +15,10 @@ RDEPENDS_${PN}-tests += "bash"
>>  
>>  PACKAGE_BEFORE_PN = "${PN}-benchmarks ${PN}-tests"
>>  
>> -SRC_URI[md5sum] = "b09d69526c86174007bea2228e36b2f1"
>> -SRC_URI[sha256sum] =
>> "3596e1d36e9c1203eed5abff5a0f272d573f4eb917b5cee4f029cce5a70a0f0c"
>> +PV = "1.15"
>> +
>> +SRC_URI[md5sum] = "dea5d3a60f93054760d773aaa3a36b1f"
>> +SRC_URI[sha256sum] =
>> "dcf256112d2692aa49ce6c6d17a898f60b3709b98c2f1c4592b23d22371462bd"
>>  
>>  PACKAGECONFIG ??= ""
>>  PACKAGECONFIG[libunwind] = "--with-libunwind,--without-
>> libunwind,libunwind,libunwind"
>> -- 
>> 2.7.4
>> 

-- 
___
meta-intel mailing list
meta-intel@yoctoproject.org
https://lists.yoctoproject.org/listinfo/meta-intel

Re: [meta-intel] [PATCH] IGT: update 1.14 -> 1.15

[Saul Wold]

On Thu, 2016-09-08 at 17:02 -0700, Jianxun Zhang wrote:
> 
> This change update intel-gpu-tools to 1.15 as a sync-up with
> Intel graphic stack 2016 Q2 release.
> 
> This change explicitly sets ${PV} in recipe and renames it
> after package name only. By doing so we don't need to enforce
> a policy to rename recipe every time we do update. Patch
> speaks itself.
> 
This is wrong!  

The whole point of naming the recipe as ${PN}_${PV} is to remove the
need to update PV in the recipe.

Yes checksums need to be updated on a regular basis, there are tools
like devtool that can assist with this.

The OpenEmbedded standard is for $PN_$PV.bb filename this also assists
in understand quickly what version a particular recipe is.  There are
also limited cases where 2 different $PVs are needed for some reasons.

Please resubmit this as a normal update with $PV contained in the file
name.

Please see: https://wiki.yoctoproject.org/wiki/Best_Known_Methods_(BKMs
)_for_Package_Updating

Thanks 
   Sau!


> 
> ${PV} and checksums shall be updated together. When either
> checksums or ${PV} is forgotten to update, developers will
> get a build error as a heads-up. This is because graphic base
> in OE fetches source with ${PV}.
> 
> Signed-off-by: Jianxun Zhang 
> ---
> Rough test is done by cherry-picking some IGT tools and tests.
> I still need to hack piglit to run IGT test as an authentic test
> approach, but I will address this with another patch to OE.
> 
>  .../intel-gpu-tools/{intel-gpu-tools_1.14.bb => intel-gpu-tools.bb}
> > 
> > 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
>  rename common/recipes-graphics/intel-gpu-tools/{intel-gpu-
> tools_1.14.bb => intel-gpu-tools.bb} (88%)
> 
> diff --git a/common/recipes-graphics/intel-gpu-tools/intel-gpu-
> tools_1.14.bb b/common/recipes-graphics/intel-gpu-tools/intel-gpu-
> tools.bb
> similarity index 88%
> rename from common/recipes-graphics/intel-gpu-tools/intel-gpu-
> tools_1.14.bb
> rename to common/recipes-graphics/intel-gpu-tools/intel-gpu-tools.bb
> index c20ce31..64cc612 100644
> --- a/common/recipes-graphics/intel-gpu-tools/intel-gpu-tools_1.14.bb
> +++ b/common/recipes-graphics/intel-gpu-tools/intel-gpu-tools.bb
> @@ -15,8 +15,10 @@ RDEPENDS_${PN}-tests += "bash"
>  
>  PACKAGE_BEFORE_PN = "${PN}-benchmarks ${PN}-tests"
>  
> -SRC_URI[md5sum] = "b09d69526c86174007bea2228e36b2f1"
> -SRC_URI[sha256sum] =
> "3596e1d36e9c1203eed5abff5a0f272d573f4eb917b5cee4f029cce5a70a0f0c"
> +PV = "1.15"
> +
> +SRC_URI[md5sum] = "dea5d3a60f93054760d773aaa3a36b1f"
> +SRC_URI[sha256sum] =
> "dcf256112d2692aa49ce6c6d17a898f60b3709b98c2f1c4592b23d22371462bd"
>  
>  PACKAGECONFIG ??= ""
>  PACKAGECONFIG[libunwind] = "--with-libunwind,--without-
> libunwind,libunwind,libunwind"
> -- 
> 2.7.4
> 
-- 
___
meta-intel mailing list
meta-intel@yoctoproject.org
https://lists.yoctoproject.org/listinfo/meta-intel