Re: Problem with uploading consecutive diffs with updates
I think there's a misunderstanding of the problem. If viewing a single diff (not comparing against two) is showing extra stuff in your change, then something is going wrong in your Perforce client, causing it to report improper revisions when building the diff. I can't say what that is, because it's something I've never heard of happening before (and perforce is very heavily used with Review Board). We're dependent on that revision information being correct, since that's what we fetch from the Perforce server when rendering the diff. We have no control over that information. The interdiffs are a whole different matter. An interdiff is a comparison between two sets of patched files, so we're dependent on whatever was uploaded. If that's a patch on revision 5 and another on revision 10, and changes have been made to that file between those revisions, we're going to get those changes because that's what a diff between those revisions will show. We don't know any better. We can't just sync to one revision and apply both patches, because a good amount of the time, that just doesn't work. This is no different of a problem than a developer's standard sync/resolve cycle. Imagine if that can be automated 100% of the time, correctly. It can't, really, because the correct result is not mathematical but rather dependent on what the user intends as the correctly resolved set of changes. We'd need to get this right, somehow, to show these differences properly. There are potentially things that can be done to better weed out parts of the diff for unrelated chunks of code, but that's a pretty hard diff analyses problem and I don't know that anyone's solved it. Certainly, we have not, and I haven't seen solutions out there. Even if you get those right, you're still going to inevitably get things wrong within the vicinity of code you do want to see. And you may even lose that code, which defeats the entire purpose. Believe me, we've thought about this a lot and right now, unless someone comes up with a model that hasn't been explored, I'm saying that given what information Review Board knows and what we can do and the algorithms available, this can't be fixed. Maybe that will change someday, but today that's my answer. Christian -- Christian Hammond - chip...@chipx86.com Review Board - http://www.reviewboard.org VMware, Inc. - http://www.vmware.com On Tue, May 15, 2012 at 8:46 AM, Robert Dailey wrote: > This happens outside of interdiff too. There is never a reason to say > something can't be fixed. It may be difficult, but not impossible. > > Either review-board or post-review has access to file revisions in > Perforce. All you need to do is simply set your base (original) in the diff > to be locked to the latest revision of the file (or at least match the base > revision that is in my changelist). post-review can check this prior to > updating an existing review, so there's no reason this can't be done. I > don't know the logistics of how Review Board or post-review works, but in > theory this is a solvable problem. > > > On Mon, May 14, 2012 at 8:12 PM, Christian Hammond wrote: > >> Oh! The interdiff was the key thing. >> >> So, yes, viewing a diff between uploaded diffs will give you that result. >> There's really not anything we can do about this. What that is really doing >> is taking the patched set of files in r1 and diffing them against the >> patched set in r2. Since there are changes in r2 that came about from the >> sync, you're going to get some junk in there. >> >> It's annoying, but we basically don't know any better. There's a bug open >> about this, but it'll probably stay open forever. >> >> One option that some people use (it's a bit manual, but...) is to sync >> with the same changes in r1 and post that up, and *then* put the changes >> from r2 up. However, this is kind of time-consuming and annoying without >> some patch management system inbetween (like quilt or Git). >> >> >> Christian >> >> -- >> Christian Hammond - chip...@chipx86.com >> Review Board - http://www.reviewboard.org >> VMware, Inc. - http://www.vmware.com >> >> >> On Mon, May 14, 2012 at 10:28 AM, Robert Dailey > > wrote: >> >>> When I say "get latest" I'm referring to the "Get Latest Revision" >>> option in P4V. I don't use p4 directly, so I'm not sure what command it >>> maps to. >>> >>> Right click in your workspace view on the folder you want to get latest >>> changes, and click "Get Latest Revision". This will pull down the latest >>> changes in perforce to files. >>> >>> Your description is almost right. I've corrected it below: >>> >>> You have a changelist with some code in it. You sync, new changes are >>> pulled in. You *re-*post the *existing* changeset to Review Board, and >>> the diff is including the things that came from the sync? >>> >>> This is absolutely correct. Note that I'm only working with 1 >>> changelist. I update my existing review with the same changelist a second >>> time, and any updates pulled f
Re: Problem with uploading consecutive diffs with updates
This happens outside of interdiff too. There is never a reason to say something can't be fixed. It may be difficult, but not impossible. Either review-board or post-review has access to file revisions in Perforce. All you need to do is simply set your base (original) in the diff to be locked to the latest revision of the file (or at least match the base revision that is in my changelist). post-review can check this prior to updating an existing review, so there's no reason this can't be done. I don't know the logistics of how Review Board or post-review works, but in theory this is a solvable problem. On Mon, May 14, 2012 at 8:12 PM, Christian Hammond wrote: > Oh! The interdiff was the key thing. > > So, yes, viewing a diff between uploaded diffs will give you that result. > There's really not anything we can do about this. What that is really doing > is taking the patched set of files in r1 and diffing them against the > patched set in r2. Since there are changes in r2 that came about from the > sync, you're going to get some junk in there. > > It's annoying, but we basically don't know any better. There's a bug open > about this, but it'll probably stay open forever. > > One option that some people use (it's a bit manual, but...) is to sync > with the same changes in r1 and post that up, and *then* put the changes > from r2 up. However, this is kind of time-consuming and annoying without > some patch management system inbetween (like quilt or Git). > > > Christian > > -- > Christian Hammond - chip...@chipx86.com > Review Board - http://www.reviewboard.org > VMware, Inc. - http://www.vmware.com > > > On Mon, May 14, 2012 at 10:28 AM, Robert Dailey > wrote: > >> When I say "get latest" I'm referring to the "Get Latest Revision" option >> in P4V. I don't use p4 directly, so I'm not sure what command it maps to. >> >> Right click in your workspace view on the folder you want to get latest >> changes, and click "Get Latest Revision". This will pull down the latest >> changes in perforce to files. >> >> Your description is almost right. I've corrected it below: >> >> You have a changelist with some code in it. You sync, new changes are >> pulled in. You *re-*post the *existing* changeset to Review Board, and >> the diff is including the things that came from the sync? >> >> This is absolutely correct. Note that I'm only working with 1 changelist. >> I update my existing review with the same changelist a second time, and any >> updates pulled from the perforce server (that other people made) will >> appear in the diff on Review Board. Also please note that all I'm doing is >> running the post-review command twice: >> >> 1) Create a new changelist, modify some source code >> 2) Use that post-review command to create a new review on Review Board. >> 3) Run "Get Latest Revision" in P4V to pull down latest source code >> changes. Files previously put up for review are modified by the sync. I >> resolve any conflicts and make more changes to the source files. >> 4) Run the same post-review command a second time, same one as before, >> same changelist number >> 5) View the diff online at Review Board server, and do a diff between r1 >> and r2, in this diff I see the changes I made in r2 as well as the diff for >> code changes pulled down from the Perforce server (those are the changes I >> did not make). >> >> Hope this makes more sense. >> >> Is this a problem with post-review, or am I simply not running the right >> commands? >> >> >> On Sun, May 13, 2012 at 5:48 PM, Christian Hammond >> wrote: >> >>> Hi Robert, >>> >>> Sorry for the late reply. I've been on vacation. >>> >>> I'm not sure I'm fully understanding the problem you're hitting. Let me >>> see if I have this right. You have a changelist with some code in it. You >>> sync, new changes are pulled in. You post the new changeset to Review >>> Board, and the diff is including the things that came from the sync? >>> >>> I've never seen this behavior, and use Review Board with Perforce almost >>> daily. Posting a new review request *should* include only what changed for >>> those files. >>> >>> When you say a "get latest", is this 'p4 sync', or something else? >>> >>> This sounds like this is a client-side problem, where the changes aren't >>> resolved properly or the metadata isn't being updated properly, causing the >>> revision for the files to be incorrect. >>> >>> Christian >>> >>> -- >>> Christian Hammond - chip...@chipx86.com >>> Review Board - http://www.reviewboard.org >>> VMware, Inc. - http://www.vmware.com >>> >>> >>> On Mon, May 7, 2012 at 8:39 AM, Robert Dailey >>> wrote: >>> Can someone help me out here? On Wed, May 2, 2012 at 1:37 PM, Robert Dailey >>> > wrote: > So I'm using Perforce and I upload my initial reviews (as well as > follow up changes and updates) using this command. Note that this is a > "Custom Tool" in P4V: > > %C --server=http://reviewboard.company.com --p4-client=$c > --p4-
Re: Problem with uploading consecutive diffs with updates
Oh! The interdiff was the key thing. So, yes, viewing a diff between uploaded diffs will give you that result. There's really not anything we can do about this. What that is really doing is taking the patched set of files in r1 and diffing them against the patched set in r2. Since there are changes in r2 that came about from the sync, you're going to get some junk in there. It's annoying, but we basically don't know any better. There's a bug open about this, but it'll probably stay open forever. One option that some people use (it's a bit manual, but...) is to sync with the same changes in r1 and post that up, and *then* put the changes from r2 up. However, this is kind of time-consuming and annoying without some patch management system inbetween (like quilt or Git). Christian -- Christian Hammond - chip...@chipx86.com Review Board - http://www.reviewboard.org VMware, Inc. - http://www.vmware.com On Mon, May 14, 2012 at 10:28 AM, Robert Dailey wrote: > When I say "get latest" I'm referring to the "Get Latest Revision" option > in P4V. I don't use p4 directly, so I'm not sure what command it maps to. > > Right click in your workspace view on the folder you want to get latest > changes, and click "Get Latest Revision". This will pull down the latest > changes in perforce to files. > > Your description is almost right. I've corrected it below: > > You have a changelist with some code in it. You sync, new changes are > pulled in. You *re-*post the *existing* changeset to Review Board, and > the diff is including the things that came from the sync? > > This is absolutely correct. Note that I'm only working with 1 changelist. > I update my existing review with the same changelist a second time, and any > updates pulled from the perforce server (that other people made) will > appear in the diff on Review Board. Also please note that all I'm doing is > running the post-review command twice: > > 1) Create a new changelist, modify some source code > 2) Use that post-review command to create a new review on Review Board. > 3) Run "Get Latest Revision" in P4V to pull down latest source code > changes. Files previously put up for review are modified by the sync. I > resolve any conflicts and make more changes to the source files. > 4) Run the same post-review command a second time, same one as before, > same changelist number > 5) View the diff online at Review Board server, and do a diff between r1 > and r2, in this diff I see the changes I made in r2 as well as the diff for > code changes pulled down from the Perforce server (those are the changes I > did not make). > > Hope this makes more sense. > > Is this a problem with post-review, or am I simply not running the right > commands? > > > On Sun, May 13, 2012 at 5:48 PM, Christian Hammond wrote: > >> Hi Robert, >> >> Sorry for the late reply. I've been on vacation. >> >> I'm not sure I'm fully understanding the problem you're hitting. Let me >> see if I have this right. You have a changelist with some code in it. You >> sync, new changes are pulled in. You post the new changeset to Review >> Board, and the diff is including the things that came from the sync? >> >> I've never seen this behavior, and use Review Board with Perforce almost >> daily. Posting a new review request *should* include only what changed for >> those files. >> >> When you say a "get latest", is this 'p4 sync', or something else? >> >> This sounds like this is a client-side problem, where the changes aren't >> resolved properly or the metadata isn't being updated properly, causing the >> revision for the files to be incorrect. >> >> Christian >> >> -- >> Christian Hammond - chip...@chipx86.com >> Review Board - http://www.reviewboard.org >> VMware, Inc. - http://www.vmware.com >> >> >> On Mon, May 7, 2012 at 8:39 AM, Robert Dailey >> wrote: >> >>> Can someone help me out here? >>> >>> >>> On Wed, May 2, 2012 at 1:37 PM, Robert Dailey >>> wrote: >>> So I'm using Perforce and I upload my initial reviews (as well as follow up changes and updates) using this command. Note that this is a "Custom Tool" in P4V: %C --server=http://reviewboard.company.com --p4-client=$c --p4-port=$p --username=user --password=password %C is the changelist number in Perforce $c is the current workspace $p is the current port The problem I'm experiencing is updates to code in my changelist being included in my diffs in reviewboard. So suppose I have files checked out in my changelist. If I do a "get latest" from Perforce, and some of those checked out files get changed, and I resolve conflicts, those updates I pulled down will now show up in my diff. Reviewboard should not be showing these changes in my diff, in other words, Reviewboard needs to use the latest base of each file when diffing, which it does not. It continues to use the same revisions of the files that it originally used when the review was first cr
Re: Problem with uploading consecutive diffs with updates
When I say "get latest" I'm referring to the "Get Latest Revision" option in P4V. I don't use p4 directly, so I'm not sure what command it maps to. Right click in your workspace view on the folder you want to get latest changes, and click "Get Latest Revision". This will pull down the latest changes in perforce to files. Your description is almost right. I've corrected it below: You have a changelist with some code in it. You sync, new changes are pulled in. You *re-*post the *existing* changeset to Review Board, and the diff is including the things that came from the sync? This is absolutely correct. Note that I'm only working with 1 changelist. I update my existing review with the same changelist a second time, and any updates pulled from the perforce server (that other people made) will appear in the diff on Review Board. Also please note that all I'm doing is running the post-review command twice: 1) Create a new changelist, modify some source code 2) Use that post-review command to create a new review on Review Board. 3) Run "Get Latest Revision" in P4V to pull down latest source code changes. Files previously put up for review are modified by the sync. I resolve any conflicts and make more changes to the source files. 4) Run the same post-review command a second time, same one as before, same changelist number 5) View the diff online at Review Board server, and do a diff between r1 and r2, in this diff I see the changes I made in r2 as well as the diff for code changes pulled down from the Perforce server (those are the changes I did not make). Hope this makes more sense. Is this a problem with post-review, or am I simply not running the right commands? On Sun, May 13, 2012 at 5:48 PM, Christian Hammond wrote: > Hi Robert, > > Sorry for the late reply. I've been on vacation. > > I'm not sure I'm fully understanding the problem you're hitting. Let me > see if I have this right. You have a changelist with some code in it. You > sync, new changes are pulled in. You post the new changeset to Review > Board, and the diff is including the things that came from the sync? > > I've never seen this behavior, and use Review Board with Perforce almost > daily. Posting a new review request *should* include only what changed for > those files. > > When you say a "get latest", is this 'p4 sync', or something else? > > This sounds like this is a client-side problem, where the changes aren't > resolved properly or the metadata isn't being updated properly, causing the > revision for the files to be incorrect. > > Christian > > -- > Christian Hammond - chip...@chipx86.com > Review Board - http://www.reviewboard.org > VMware, Inc. - http://www.vmware.com > > > On Mon, May 7, 2012 at 8:39 AM, Robert Dailey wrote: > >> Can someone help me out here? >> >> >> On Wed, May 2, 2012 at 1:37 PM, Robert Dailey >> wrote: >> >>> So I'm using Perforce and I upload my initial reviews (as well as follow >>> up changes and updates) using this command. Note that this is a "Custom >>> Tool" in P4V: >>> >>> %C --server=http://reviewboard.company.com --p4-client=$c --p4-port=$p >>> --username=user --password=password >>> >>> %C is the changelist number in Perforce >>> $c is the current workspace >>> $p is the current port >>> >>> The problem I'm experiencing is updates to code in my changelist being >>> included in my diffs in reviewboard. So suppose I have files checked out in >>> my changelist. If I do a "get latest" from Perforce, and some of those >>> checked out files get changed, and I resolve conflicts, those updates I >>> pulled down will now show up in my diff. Reviewboard should not be showing >>> these changes in my diff, in other words, Reviewboard needs to use the >>> latest base of each file when diffing, which it does not. It continues to >>> use the same revisions of the files that it originally used when the review >>> was first created. >>> >>> Is there a fix for this? >>> >> >> -- >> Want to help the Review Board project? Donate today at >> http://www.reviewboard.org/donate/ >> Happy user? Let us know at http://www.reviewboard.org/users/ >> -~--~~~~--~~--~--~--- >> To unsubscribe from this group, send email to >> reviewboard+unsubscr...@googlegroups.com >> For more options, visit this group at >> http://groups.google.com/group/reviewboard?hl=en >> > > -- > Want to help the Review Board project? Donate today at > http://www.reviewboard.org/donate/ > Happy user? Let us know at http://www.reviewboard.org/users/ > -~--~~~~--~~--~--~--- > To unsubscribe from this group, send email to > reviewboard+unsubscr...@googlegroups.com > For more options, visit this group at > http://groups.google.com/group/reviewboard?hl=en -- Want to help the Review Board project? Donate today at http://www.reviewboard.org/donate/ Happy user? Let us know at http://www.reviewboard.org/users/ -~--~~~~--~~--~--~--- To unsubscribe from this gro
Re: Problem with uploading consecutive diffs with updates
Hi Robert, Sorry for the late reply. I've been on vacation. I'm not sure I'm fully understanding the problem you're hitting. Let me see if I have this right. You have a changelist with some code in it. You sync, new changes are pulled in. You post the new changeset to Review Board, and the diff is including the things that came from the sync? I've never seen this behavior, and use Review Board with Perforce almost daily. Posting a new review request *should* include only what changed for those files. When you say a "get latest", is this 'p4 sync', or something else? This sounds like this is a client-side problem, where the changes aren't resolved properly or the metadata isn't being updated properly, causing the revision for the files to be incorrect. Christian -- Christian Hammond - chip...@chipx86.com Review Board - http://www.reviewboard.org VMware, Inc. - http://www.vmware.com On Mon, May 7, 2012 at 8:39 AM, Robert Dailey wrote: > Can someone help me out here? > > > On Wed, May 2, 2012 at 1:37 PM, Robert Dailey wrote: > >> So I'm using Perforce and I upload my initial reviews (as well as follow >> up changes and updates) using this command. Note that this is a "Custom >> Tool" in P4V: >> >> %C --server=http://reviewboard.company.com --p4-client=$c --p4-port=$p >> --username=user --password=password >> >> %C is the changelist number in Perforce >> $c is the current workspace >> $p is the current port >> >> The problem I'm experiencing is updates to code in my changelist being >> included in my diffs in reviewboard. So suppose I have files checked out in >> my changelist. If I do a "get latest" from Perforce, and some of those >> checked out files get changed, and I resolve conflicts, those updates I >> pulled down will now show up in my diff. Reviewboard should not be showing >> these changes in my diff, in other words, Reviewboard needs to use the >> latest base of each file when diffing, which it does not. It continues to >> use the same revisions of the files that it originally used when the review >> was first created. >> >> Is there a fix for this? >> > > -- > Want to help the Review Board project? Donate today at > http://www.reviewboard.org/donate/ > Happy user? Let us know at http://www.reviewboard.org/users/ > -~--~~~~--~~--~--~--- > To unsubscribe from this group, send email to > reviewboard+unsubscr...@googlegroups.com > For more options, visit this group at > http://groups.google.com/group/reviewboard?hl=en > -- Want to help the Review Board project? Donate today at http://www.reviewboard.org/donate/ Happy user? Let us know at http://www.reviewboard.org/users/ -~--~~~~--~~--~--~--- To unsubscribe from this group, send email to reviewboard+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/reviewboard?hl=en
Re: Problem with uploading consecutive diffs with updates
Can someone help me out here? On Wed, May 2, 2012 at 1:37 PM, Robert Dailey wrote: > So I'm using Perforce and I upload my initial reviews (as well as follow > up changes and updates) using this command. Note that this is a "Custom > Tool" in P4V: > > %C --server=http://reviewboard.company.com --p4-client=$c --p4-port=$p > --username=user --password=password > > %C is the changelist number in Perforce > $c is the current workspace > $p is the current port > > The problem I'm experiencing is updates to code in my changelist being > included in my diffs in reviewboard. So suppose I have files checked out in > my changelist. If I do a "get latest" from Perforce, and some of those > checked out files get changed, and I resolve conflicts, those updates I > pulled down will now show up in my diff. Reviewboard should not be showing > these changes in my diff, in other words, Reviewboard needs to use the > latest base of each file when diffing, which it does not. It continues to > use the same revisions of the files that it originally used when the review > was first created. > > Is there a fix for this? > -- Want to help the Review Board project? Donate today at http://www.reviewboard.org/donate/ Happy user? Let us know at http://www.reviewboard.org/users/ -~--~~~~--~~--~--~--- To unsubscribe from this group, send email to reviewboard+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/reviewboard?hl=en
