Re: [PATCH 1/1] git-p4: unshelve: use action==add instead of rev==none
>> 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
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
> 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
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
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
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.