git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v3 0/2] cvsimport: add test illustrating a bug in cvsps

From
Junio C Hamano <gitster@pobox.com>
Date
Mar 18, 2009, 18:22 UTC
Message-ID
<7vfxhag07g.fsf@gitster.siamese.dyndns.org>
In-Reply-To
<49C13062.4020400@hvoigt.net>
Heiko Voigt <git-list@hvoigt.net> writes:
> This is an updated version of the first patch and an addition to ensure
> correct handling of branches in fixes. 

I've already merged the first one to 'next' so this needs to be turned into an incremental update if we were to continue building on top in the git project.

I however have a bigger issue with this, perhaps because I do not have the feeling that I fully understand where these patches are going.

Your approach seems to me to:
 - add tests to git test suite that expose issues the current cvsimport
   that runs on an unpatched cvsps has;
 - diagnose and fix
   - the issues in cvsimport, if the problem is because cvsimport is
     mishandling correct output from cvsps; or
   - the issues in cvsps (and adjust cvsimport to patched cvsps if
     necessary), if the problem is because output from cvsps is incorrect.

That all feels sane, and having the tests to verify the end result would help the people who collaborate on these tasks.

But how much of the actual fix will be made to cvsps, and how much to cvsimport? If the majority of the changes are to happen on cvsps (which is not unexpected, given that many people who tried and wrote various cvs importers put blame on the shortcomings of its output), I am afraid that it would not help majority of git users until the fixes to cvsps that come out of this effort hit their distros for me to keep these tests in the git.git repository. I do not build and install custom cvsps (because I haven't had to work with complex history in CVS that your improvements to cvsps are need to deal with correctly), and I suspect many others are in the same boat. In addition, if your tests are in the git.git repository, they need to say test_expect_success for people with patched cvsps and test_expect_failure for people without, and because I suspect that the majority of git developers do not run bleeding edge cvsps, it does not do anything but slowing down the test suite.

It feels as if you are scratching my feet through my shoes while I still am wearing them. I wonder if it would be more direct and simpler approach to add tests to cvsps and handle these improvements as part of the cvsps maintenance/development effort, not as part of cvsimport fixes, at least initially.

I think it is great that you started actively working on identifying and fixing issues with cvsps, that many others have gave up and gone to different avenues, and I certainly do not mind keeping the new tests in 'pu' for wider exposure, in order to make it easier for other people who use cvsimport and want to collaborate with you improving it through improving cvsps.

But I am starting to think that it was a mistake on my part to have merged the initial set of tests to 'next'.

Thoughts?
Previous: Heiko VoigtNext: Michael J Gruber
Message 9 of 21 in “cvsimport: add test illustrating a bug in cvsps”
  1. cvsimport: add test illustrating a bug in cvspsHeiko Voigt, Feb 23, 2009
  2. Heiko VoigtFeb 23, 2009
  3. Michael HaggertyFeb 24, 2009
  4. 0/1 cvsimport: add test illustrating a bug in cvspsHeiko Voigt, Mar 2, 2009
  5. 1/1 cvsimport: add test illustrating a bug in cvspsHeiko Voigt, Mar 2, 2009
  6. fix: correct rev order in case commiters clocks were not syncronisedHeiko Voigt, Mar 9, 2009
  7. Michael HaggertyMar 9, 2009
  8. 0/2 cvsimport: add test illustrating a bug in cvspsHeiko Voigt, Mar 18, 2009
  9. Junio C HamanoMar 18, 2009
  10. Michael J GruberMar 19, 2009
  11. Johannes SchindelinMar 19, 2009
  12. Michael J GruberMar 19, 2009
  13. Michael HaggertyMar 21, 2009
  14. started a cvsps testsuite Was: Re: [PATCH v3 0/2] cvsimport: add test illustrating a bug in cvspsHeiko Voigt, Mar 23, 2009
  15. Martin LanghoffMar 23, 2009
  16. Michael HaggertyMar 24, 2009
  17. Heiko VoigtApr 6, 2009
  18. Heiko VoigtMar 23, 2009
  19. 1/2 cvsimport: add test illustrating a bug in cvspsHeiko Voigt, Mar 18, 2009
  20. 2/2 cvsimport: extend testcase about patchset order to contain branchesHeiko Voigt, Mar 18, 2009
  21. fix: correct rev order in case commiters clocks were not syncronisedHeiko Voigt, Mar 18, 2009

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.