Re: [PATCH 1/1] git-p4: unshelve: use action==add instead of rev==none

[Mazo, Andrey]

>> This was actually discussed in a separate thread [1] some time ago with 
>> patches proposed by Thandesha and me.
>> I haven't yet got time to cook a final patch, which addresses both 
>> Thandesha's and mine use-cases though,
>> so this wasn't submitted to Junio yet.
>> In the meantime, I guess, one of the patches [2] from that thread can be 
>> taken as is.
>>
>> [1] "[BUG] git p4 clone fails when p4 sizes does not return 'fileSize' key"
>>    
>>https://public-inbox.org/git/cajjpmi-plb4qcka5alkxa8b1vozfff+oaq0fguq9yviobrp...@mail.gmail.com/t/#mee2ec50a40242089741f808f06214a44278055b3
>> [2] "[PATCH 1/1] git-p4: fix `sync --verbose` traceback due to 'fileSize'"
>>    
>>https://public-inbox.org/git/2e2b2add4e4fffa4228b8ab9f6cd47fa9bf25207.1523981210.git.am...@checkvideo.com/
>
> Should I re-roll my patch without this change then?
If it's the question to me, then I'm fine either way -- I can rebase my changes 
easily.
However, re-rolling your patch would probably make the aforementioned fileSize 
patch apply cleanly to both maint and master branches.


Thank you,
Andrey Mazo

Re: [PATCH 1/1] git-p4: unshelve: use action==add instead of rev==none

[Luke Diamand]

On 23 May 2018 at 17:41, Mazo, Andrey  wrote:
>> The last one (i.e. "even if it is verbose, if fileSize is not
>> reported, do not write the verbose output") does not look like it is
>> limited to the unshelve feature, so it might, even though it is a
>> one-liner, deserve to be a separate preparatory patch if you want.
>> But I do not feel strongly about either way.
>
> This was actually discussed in a separate thread [1] some time ago with 
> patches proposed by Thandesha and me.
> I haven't yet got time to cook a final patch, which addresses both 
> Thandesha's and mine use-cases though,
> so this wasn't submitted to Junio yet.
> In the meantime, I guess, one of the patches [2] from that thread can be 
> taken as is.
>
> [1] "[BUG] git p4 clone fails when p4 sizes does not return 'fileSize' key"
>   
> https://public-inbox.org/git/cajjpmi-plb4qcka5alkxa8b1vozfff+oaq0fguq9yviobrp...@mail.gmail.com/t/#mee2ec50a40242089741f808f06214a44278055b3
> [2] "[PATCH 1/1] git-p4: fix `sync --verbose` traceback due to 'fileSize'"
>   
> https://public-inbox.org/git/2e2b2add4e4fffa4228b8ab9f6cd47fa9bf25207.1523981210.git.am...@checkvideo.com/

Should I re-roll my patch without this change then?

Luke

Re: [PATCH 1/1] git-p4: unshelve: use action==add instead of rev==none

[Mazo, Andrey]

> The last one (i.e. "even if it is verbose, if fileSize is not
> reported, do not write the verbose output") does not look like it is
> limited to the unshelve feature, so it might, even though it is a
> one-liner, deserve to be a separate preparatory patch if you want.
> But I do not feel strongly about either way.

This was actually discussed in a separate thread [1] some time ago with patches 
proposed by Thandesha and me.
I haven't yet got time to cook a final patch, which addresses both Thandesha's 
and mine use-cases though,
so this wasn't submitted to Junio yet.
In the meantime, I guess, one of the patches [2] from that thread can be taken 
as is.

[1] "[BUG] git p4 clone fails when p4 sizes does not return 'fileSize' key"
  
https://public-inbox.org/git/cajjpmi-plb4qcka5alkxa8b1vozfff+oaq0fguq9yviobrp...@mail.gmail.com/t/#mee2ec50a40242089741f808f06214a44278055b3
[2] "[PATCH 1/1] git-p4: fix `sync --verbose` traceback due to 'fileSize'"
  
https://public-inbox.org/git/2e2b2add4e4fffa4228b8ab9f6cd47fa9bf25207.1523981210.git.am...@checkvideo.com/

Thank you,
Andrey Mazo

Re: [PATCH 1/1] git-p4: unshelve: use action==add instead of rev==none

[Junio C Hamano]

Luke Diamand  writes:

>> However, instead of a separate patch, wouldn't it be better to squash
>> it into the previous one?  So 'make test' would succeed on every
>> commit even with a newer p4 version.
>
> Junio?
>
> I can squash together the original commit and the two fixes if that
> would be better?

Among the three hunks in this fix-up patch, the first two are
strictly fixing what you had in the previous patch, so it make sense
to fix them at the source by squashing.

The last one (i.e. "even if it is verbose, if fileSize is not
reported, do not write the verbose output") does not look like it is
limited to the unshelve feature, so it might, even though it is a
one-liner, deserve to be a separate preparatory patch if you want.
But I do not feel strongly about either way.

Thanks.

Re: [PATCH 1/1] git-p4: unshelve: use action==add instead of rev==none

[Luke Diamand]

On 22 May 2018 at 11:15, SZEDER Gábor  wrote:
> On Tue, May 22, 2018 at 10:41 AM, Luke Diamand  wrote:
>> SZEDER Gábor found that the unshelve tests fail with newer
>> versions of Perforce (2016 vs 2015).
>>
>> The problem arises because when a file is added in a P4
>> shelved changelist, the depot revision is shown as "none"
>> if using the older p4d (which makes sense - the file doesn't
>> yet exist, so can't have a revision), but as "1" in the newer
>> versions of p4d.
>>
>> For example, adding a file called "new" with 2015.1 and then
>> shelving that change gives this from "p4 describe" :
>>
>> ... //depot/new#none add
>>
>> Using the 2018.1 server gives this:
>>
>> ... //depot/new#1 add
>>
>> We can detect that a file has been added simply by using the
>> file status ("add") instead, rather than the depot revision,
>> which is what this change does.
>>
>> This also fixes a few verbose prints used for debugging this
>> to be more friendly.
>>
>> Signed-off-by: Luke Diamand 
>
> For what it's worth, I can confirm that 't9832-unshelve.sh' passes
> with these changes, here and in all Linux and OSX build jobs on Travis
> CI.

Thanks!

>
> However, instead of a separate patch, wouldn't it be better to squash
> it into the previous one?  So 'make test' would succeed on every
> commit even with a newer p4 version.

Junio?

I can squash together the original commit and the two fixes if that
would be better?

Thanks!
Luke

Re: [PATCH 1/1] git-p4: unshelve: use action==add instead of rev==none

[SZEDER Gábor]

On Tue, May 22, 2018 at 10:41 AM, Luke Diamand  wrote:
> SZEDER Gábor found that the unshelve tests fail with newer
> versions of Perforce (2016 vs 2015).
>
> The problem arises because when a file is added in a P4
> shelved changelist, the depot revision is shown as "none"
> if using the older p4d (which makes sense - the file doesn't
> yet exist, so can't have a revision), but as "1" in the newer
> versions of p4d.
>
> For example, adding a file called "new" with 2015.1 and then
> shelving that change gives this from "p4 describe" :
>
> ... //depot/new#none add
>
> Using the 2018.1 server gives this:
>
> ... //depot/new#1 add
>
> We can detect that a file has been added simply by using the
> file status ("add") instead, rather than the depot revision,
> which is what this change does.
>
> This also fixes a few verbose prints used for debugging this
> to be more friendly.
>
> Signed-off-by: Luke Diamand 

For what it's worth, I can confirm that 't9832-unshelve.sh' passes
with these changes, here and in all Linux and OSX build jobs on Travis
CI.

However, instead of a separate patch, wouldn't it be better to squash
it into the previous one?  So 'make test' would succeed on every
commit even with a newer p4 version.