[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Proposed branch tag performance patch for feature

From: Mark D. Baushke
Subject: Re: Proposed branch tag performance patch for feature
Date: Thu, 04 Jan 2007 08:22:49 -0800

Hash: SHA1

Kelly F. Hickel <kfh@mqsoftware.com> writes:

> The not as good news, quite possibly to the above mentioned non-groking
> of CVS_LOCAL_BRANCH_NUM, it looks to me that the feature patch is
> probably wrong.  Looks like your using that to set the defaultrv param
> to findnextmagicrev, but that parameter is only used if
> findnextmagicrev_proc returns an empty list (no candidate revisions, or
> some error). I had imagined that the patch for the feature release would
> have to go through the sorted list of revisions and find the highest
> value that was less than CVS_LOCAL_BRANCH_NUM and start there.  The
> other possibility that I came up with (in case I flipped the meaning of
> CVS_LOCAL_BRANCH_NUM) was that, if the highest found revision was less
> than CVS_LOCAL_BRANCH_NUM, then I'd have to start from
> CVS_LOCAL_BRANCH_NUM.  It hinges on whether CVS_LOCAL_BRANCH_NUM means
> that when branching on this repo one needs to create a magic branch tag
> with a revision above CVS_LOCAL_BRANCH_NUM, or if it needs to be less

Hmmm... the branch needs to be above CVS_LOCAL_BRANCH_NUM, so you are
correct that the patch as written is wrong.

One way to deal with this is to only use the findnextmagicrev function
if there is not an initial CVS_LOCAL_BRANCH_NUM from which to start.

    local_branch_num = getenv ("CVS_LOCAL_BRANCH_NUM");
    if (local_branch_num)
        rev_num = atoi (local_branch_num);
        if (rev_num < 2)
            rev_num = 2;
            rev_num &= ~1;
        /* Find the next unused magic rev,
         * if none are found, it should return 2.
        rev_num = findnextmagicrev (rcs, rev, 2);

Given that most of the time there should only be a 'few' local branches
in a CVSup mirror of the main repository, this may not be a horrible

> The way the feature patch currently is set up, most of the time
> CVS_LOCAL_BRANCH_NUM isn't going to figure in the calculation at all,
> which I suspect is the wrong thing. And, there probably needs to be a
> sanity.sh test for whichever is the correct behavior, and I really don't
> have any idea how to write that test (although I might, once the issues
> in the previous paragraph are resolved).

Yes, this will require a few more client tests.

        -- Mark
Version: GnuPG v1.4.6 (FreeBSD)


reply via email to

[Prev in Thread] Current Thread [Next in Thread]