On Wed, Oct 12, 2011 at 5:54 AM, Stefan Sperling <s...@elego.de> wrote: > On Tue, Oct 11, 2011 at 11:21:16AM -0700, Paul Burba wrote: >> On Tue, Oct 11, 2011 at 6:36 AM, Stefan Sperling <s...@elego.de> wrote: >> > On Tue, Oct 11, 2011 at 03:05:11PM +0200, Stefan Sperling wrote: >> >> So while I think your fixes should be backported to 1.7.1 ASAP, >> >> I don't think the status quo is acceptable. How do we want to move >> >> forward? >> >> >> >> For reference, here's the error message I'm getting: >> >> Hi Stefan, >> >> Yes, the error message is rather long. But as I've already said, this >> reintegrate merge is a complete and total abuse of the reintegrate >> feature. I'm not sure we can save every user from themselves if they >> insist on doing strange things....but I've already made that point, >> and it appears I'm in the minority, so I wont belabor it any further >> :-) > > I don't agree with this. I wouldn't call it "abuse" of this feature. > > The user is clearly intending to reintegrate the branch. But one of the > preconditions for reintegration isn't met. Much like trying to merge into > a mixed-revision working copy, or a working copy with local modifications. > Would you also call that "abuse"? I doubt that :) > > The user error is definitely not on purpose, and I don't see a point > in punishing users for this error by stealing 3 or more minutes of > their time gathering information of little value to them. > Note that the user performing the reintegrate merge is not necessarily > the same person who performed the cherry-picking merge which makes > --reintegrate impossible. They might simply be unaware of what happened. > >> So while I'm a weak -0 on the basic premise of your patch[1], I won't >> object to it. >> >> Paul >> >> [1] Re your present patch, the error message is created such that it >> appears we have to sync the source branch (/fs-successor-ids) up to >> HEAD: > >> But we need only catch up the branch to to youngest sync from trunk >> (r1167546) for the reintegrate to be successful: > > Right. That should be fixed by printing the last revision of the last > range which has already been merged. > > Of course, the patch isn't finished as it is. I just wanted to see how > much of a difference it would make to stop downloading the log immediately > when one missing operative revision was found. > > How about we meet in the middle? > We could have the log message callback keep track of the number of > missing operative revisions it found, and return > SVN_ERR_CLIENT_NOT_READY_TO_MERGE if a certain limit is exceeded. > In which case the error will end with " (one or more additional missing > revisions not shown, for brevity)". > Let's say we set the limit to a maximum of 42 missing operative revisions. > Because 42 is the answer to life, the universe, and everything, > and because this amount of missing revivions should fit well into the use > case you have in mind, where only a few revisions are missing and the > user would like to see them all at once without running the 'svn > mergeinfo' command. What do you think?
Hi Stefan, Your proposition is a good compromise. It saves users in your use case from a lengthy wait for a ginormous (and largely useless) error message but preserves the current benefits for the more common(?) use case where only a few revs are missing. 42 as the limit? Why not, it's manageable. One question though, were you thinking that 42 is the limit for *all* subtrees and if the first time that limit is exceeded we stop? Or per subtree? What I mean by this is when the reintegrate source's subtree mergeinfo doesn't reflect that it is synced with the reintegrate target. Recall that in that case the error breaks out the missing ranges by subtree. For example, say we tried something dubious like reintegrating the 1.7.x branch to trunk: C:\SVN\src-trunk-2>svn merge ^^/subversion/branches/1.7.x C:\SVN\src-trunk-2 --reintegrate ..\..\..\subversion\svn\merge-cmd.c:382: (apr_err=195016) ..\..\..\subversion\libsvn_client\merge.c:10861: (apr_err=195016) ..\..\..\subversion\libsvn_client\merge.c:10830: (apr_err=195016) ..\..\..\subversion\libsvn_client\merge.c:10830: (apr_err=195016) ..\..\..\subversion\libsvn_client\merge.c:10768: (apr_err=195016) svn: E195016: Reintegrate can only be used if revisions 1145993 through 1185721 were previously merged from https://svn.apache.org/repos/asf/subversion/trunk to the reintegrate source, but this is not the case: subversion/branches/1.7.x Missing ranges: /subversion/trunk:1145999,1146032,1146074,1146105,1146112,1146119,1146131,1146134-1146135,1146140,1146143,1146149,1146153,1146165,1146214,11 46225-1146226,1146228,1146251,1146381,1146496,1146501,1146507,1146510-1146512,1146525,1146528,1146534,1146690,1146716,1146723,1146762,1146765,1146770,1146811,11 46824,1146826-1146827,1146938,1146981-1146982,1147056,1147160,1147538,1147540-1147541,1147584-1147585,1147588-1147589,1147591,1147594,1147596,1147600,1147670,11 48043,1148134,1148395,1148426,1148434,1148436,1148448,1148453,1148502,1148512,1148536,1148573,1148576,1148593,1148619,1148629,1148644,1148830-1148831,1148884,11 48904,1148907,1148909,1148918,1148923,1148932,1148934,1148944-1148945,1148992,1149001,1149010,1149103,1149135,1149137,1149151,1149197,1149210,1149264,1149310,11 49312,1149404,1149453-1149454,1149456-1149458,1149460,1149475,1149519,1149533,1149543,1149546,1149557,1149612,1149626,1149671,1149717,1150060,1150073,1150099,11 50246,1150255,1150257,1150344,1150439,1150454,1150459,1150469,1150783,1150789,1150793,1150801,1150803,1150807,1150821,1150833,1150841-1150842,1150867-1150870,11 50872-1150875,1150883-1150884,1150924,1150950,1150952,1150954,1150958,1150961,1151015,1151020,1151044,1151053,1151055,1151068-1151069,1151073,1151166,1151239,11 51244,1151247,1151254,1151260,1151270,1151296,1151590,1151667,1151672,1151680,1151780,1151824,1151873,1151904,1151916,1151927,1151939,1151948,1151951,1151954,11 51957,1151962,1152023,1152026,1152035,1152115,1152117,1152142,1152153,1152204,1152245-1152246,1152293-1152294,1152297,1152300,1152324,1152326,1152332,1152341,11 52345,1152348,1152358,1152360,1152364,1152368,1152410,1152436,1152447,1152449,1152807,1152859,1152889,1153106,1153110,1153149,1153180,1153414,1153458,1153486,11 53527,1153539,1153599,1153618,1153839,1154060,1154070,1154388,1154598,1154700,1154913,1154938,1154946,1154948,1154968,1154976,1155000-1155001,1155279,1155325,11 56076,1156080,1156173,1156211,1156281,1156434,1156756,1156764,1156826,1156929,1156949,1156953,1157172,1157180,1157244,1157246,1157263,1157292,1157389,1157537,11 57681-1157682,1157700,1157754,1157788,1157906,1157911-1157912,1158166,1158174,1158224,1158300,1158343,1158358,1158366,1158429,1158485,1158491-1158492,1158923,11 58929,1158936,1158970,1158979,1158981,1159113-1159114,1159122,1159199-1159200,1159228,1159286,1159299,1159359,1159362,1159367,1159403,1159672,1159716,1159741,11 59751,1159775,1159873,1159880,1159924,1160204,1160210,1160212,1160215,1160217,1160318-1160320,1160601,1160604,1160615,1160625,1160630,1160645,1160672,1160691,11 60856,1160875,1161046,1161051,1161084,1161131,1161159,1161166,1161184,1161202,1161219,1161224-1161225,1161316-1161318,1161467,1161470,1161474,1161589,1161710,11 62040,1162046,1162054,1162065,1162136,1162197,1162794,1163151,1163169-1163170,1163173,1163175,1163238,1163243,1163296,1163372,1163383,1163393,1163433,1163438,11 63672,1163698,1163704,1163707,1163711,1163894,1163903,1163955,1164059,1164116,1164165,1164426,1164556,1164614,1164646,1164665,1164780-1164781,1164929,1165275,11 65316,1165346,1165631,1165713,1165742,1165787,1165791,1165807,1165814,1165819,1166096,1166098,1166101,1166111,1166114,1166158,1166183,1166192,1166229-1166230,11 66267,1166298,1166332,1166481,1166503,1166510,1166518,1166596,1166695,1166705,1166719,1166729,1166732,1166748,1166878,1166900,1167031,1167033,1167090,1167109,11 67123,1167150,1167168,1167197,1167228,1167304,1167306,1167461,1167483,1167542,1167546,1167616,1167681-1167682,1169717,1169755,1169770,1169799,1169806,1169837,11 69851,1169939,1169947,1169970,1170120,1170126,1170155,1170159,1170183,1170198,1170205,1170324,1170327,1170329,1170472,1170836,1170838,1170843,1170863,1170868,11 70915,1170919,1170924,1171000,1171196,1171351,1171353,1171358,1171702,1171713,1171732,1172391,1172400,1172407,1172411,1172512,1172527,1172548,1172582,1172586,11 72602,1172604,1172609,1172652,1172672,1172675,1172685,1172692,1172773,1173095,1173112,1173126,1173129,1173137,1173153,1173162,1173170,1173191,1173216,1173224,11 73235,1173238,1173240,1173271,1173574,1173642,1173667,1173750-1173751,1173759,1173807,1173809,1173935,1173942,1173992,1174039,1174041,1174043-1174044,1174046,11 74054,1174068,1174074,1174111,1174132,1174139,1174145,1174149,1174156,1174163,1174166,1174265,1174342,1174352,1174355,1174388,1174526,1174582,1174612,1174693,11 74699,1175068,1175835,1175849,1175903,1175938,1176416,1176797,1176894-1176896,1176904,1176909,1177494,1177563,1177693,1177700,1177702,1177732,1178265,1178280,11 78282,1178415,1178435,1178831,1178846,1178910,1178942,1179117,1179136,1179157,1179295,1179297,1179299-1179300,1179312,1179323,1179430,1179545,1179675,1179776,11 80154,1180628,1180724,1180771,1180868,1181090,1181110,1181215,1181232-1181233,1181326,1181609,1181676,1181721,1181727,1181800,1181849,1181853,1181857-1181858,11 81862-1181863,1181869-1181870,1182054,1182104,1182108,1182112-1182113,1182118,1182120,1182123,1182125-1182126,1182128,1182130,1182135,1182138,1182269,1182340,11 82373-1182374,1182376,1182396,1182435,1182459,1182467-1182468,1182475,1182492,1182496-1182497,1182499,1182513,1182527,1182679,1182771,1182778,1182784,1182790,11 82975,1182979,1182983-1182985,1182988,1183174 subversion/branches/1.7.x/CHANGES Missing ranges: /subversion/trunk/CHANGES:1182385 subversion/branches/1.7.x/subversion/include/private/svn_adler32.h Missing ranges: /subversion/trunk/subversion/include/private/svn_adler32.h:1146149 subversion/branches/1.7.x/subversion/include/private/svn_string_private.h Missing ranges: /subversion/trunk/subversion/include/private/svn_string_private.h:1146149 subversion/branches/1.7.x/subversion/include/private/svn_temp_serializer.h Missing ranges: /subversion/trunk/subversion/include/private/svn_temp_serializer.h:1146149 subversion/branches/1.7.x/subversion/libsvn_fs_fs/temp_serializer.c Missing ranges: /subversion/trunk/subversion/libsvn_fs_fs/temp_serializer.c:1146149 subversion/branches/1.7.x/subversion/libsvn_fs_fs/temp_serializer.h Missing ranges: /subversion/trunk/subversion/libsvn_fs_fs/temp_serializer.h:1146149 subversion/branches/1.7.x/subversion/libsvn_subr/adler32.c Missing ranges: /subversion/trunk/subversion/libsvn_subr/adler32.c:1146149 subversion/branches/1.7.x/subversion/libsvn_subr/hash.c Missing ranges: /subversion/trunk/subversion/libsvn_subr/hash.c:1146149 subversion/branches/1.7.x/subversion/libsvn_subr/svn_temp_serializer.c Missing ranges: /subversion/trunk/subversion/libsvn_subr/svn_temp_serializer.c:1146149 subversion/branches/1.7.x/subversion/tests/cmdline/basic_tests.py Missing ranges: /subversion/trunk/subversion/tests/cmdline/basic_tests.py:1171732 Would the rev limit apply separately to each subtree? Paul P.S. Sorry about the tardy reply, I was on the road last week, saw this, flagged it, and promptly forgot about it :-\ > In my patch, the current limit is 1. It would be easy to extend this to > 42 (or some other value) and adjust the error message accordingly. >