bug-cvs
[Top][All Lists]
Advanced

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

RE: Bug-cvs Digest, Vol 20, Issue 7


From: Mesfin, Alex
Subject: RE: Bug-cvs Digest, Vol 20, Issue 7
Date: Mon, 19 Sep 2005 13:34:03 -0500

 Hi
        Will somebody be kind enough to direct me where to start if I
want to modify cvs client in eclipse.


Alex 
Medtronic, INC.

-----Original Message-----
From: address@hidden
[mailto:address@hidden On
Behalf Of address@hidden
Sent: Monday, September 19, 2005 1:26 PM
To: address@hidden
Subject: Bug-cvs Digest, Vol 20, Issue 7

Send Bug-cvs mailing list submissions to
        address@hidden

To subscribe or unsubscribe via the World Wide Web, visit
        http://lists.nongnu.org/mailman/listinfo/bug-cvs
or, via email, send a message with subject or body 'help' to
        address@hidden

You can reach the person managing the list at
        address@hidden

When replying, please edit your Subject line so it is more specific than
"Re: Contents of Bug-cvs digest..."


Today's Topics:

   1. Re: CVS problem with ssh (Derek Price)
   2. Re: [task #4633] GPG-Signed Commits (Derek Price)
   3. Re: [task #4633] GPG-Signed Commits (Jim Hyslop)
   4. Re: [task #4633] GPG-Signed Commits  (Mark D. Baushke)
   5. [task #4687] Import recent diffutils. (Derek Robert Price)
   6. [task #4686] Bless 1.12.x stable. (Derek Robert Price)
   7. [patch #4441] Add importinfo trigger,     or cause import to run
      commitinfo/loginfo combination as does    commit. (Derek Robert
Price)


----------------------------------------------------------------------

Message: 1
Date: Thu, 15 Sep 2005 17:36:00 -0400
From: Derek Price <address@hidden>
Subject: Re: CVS problem with ssh
To: Paul Eggert <address@hidden>
Cc: address@hidden, Larry Jones <address@hidden>, address@hidden
Message-ID: <address@hidden>
Content-Type: text/plain; charset=ISO-8859-1

Paul Eggert wrote:

>Yes, I generated this patch:
>
>http://lists.gnu.org/archive/html/bug-cvs/2005-07/msg00034.html
>  
>

Actually, I still had that patch.  In the email I quoted, I thought you
were making reference to a different solution, that didn't rely on a
second process.  I think I have a way to do this, but I thought if you
already had code it might be a head start.  I will fall back on your
forking patch if this idea doesn't work out.

>Thanks.  I'd greatly appreciate it if you could get to the bottom of 
>the problem.  (If you can reproduce it, that's probably most of the 
>work right there....)
>  
>

The problem is recurring in nightly testing on BSDI, I believe it is,
with the current CVS sources.  It has also cropped up again in some
recent Solaris testing I've been setting up on the SourceForge Compile
Farm.  If we can install a fix and the problem doesn't recur for a week
or two, I'll be fairly satisfied that we got it, but I recall that
removing the select(fileno(std{out,err}), ...) calls in, um, I think
it's handle_m & handle_e in client.c, should make the sshstdio-6 test in
CVS's test suite to fail every time.

>One more thing.  I am still using CVS_RSH=/home/eggert/bin/ssh4cvs in 
>my environment, where ssh4cvs is the following script.  Perhaps this 
>could be put into the CVS FAQ?  Even if we fix the bug for newer
>  
>

If you'd like to enter it yourself, the new FAQ is Wikied at
<http://ximbiot.com/cvs/wiki/index.php?title=CVS_FAQ>.  Otherwise, I'll
try to make some time to enter it sometime soon.

Regards,

Derek

--
Derek R. Price
CVS Solutions Architect
Ximbiot <http://ximbiot.com>
v: +1 717.579.6168
f: +1 717.234.3125
<mailto:address@hidden>






------------------------------

Message: 2
Date: Mon, 19 Sep 2005 10:22:15 -0400
From: Derek Price <address@hidden>
Subject: Re: [task #4633] GPG-Signed Commits
To: "Mark D. Baushke" <address@hidden>
Cc: address@hidden, address@hidden, address@hidden
Message-ID: <address@hidden>
Content-Type: text/plain; charset=ISO-8859-1

One more thought on planning this feature, this is important enough to
go into the stable release series, I think, but we are awfully close to
being able to bless feature as stable anyhow.

Would there be any objections to GPG-signed commits going into stable as
things stand?

Would there be any objections to 1.12.x being blessed as stable after
adding GPG-signed commits, importing an updated diffutils, possibly
completing the commitid stuff, and maybe an RC release or two?

Regards,

Derek
-- 
Derek R. Price
CVS Solutions Architect
Ximbiot <http://ximbiot.com>
v: +1 717.579.6168
f: +1 717.234.3125
<mailto:address@hidden>






------------------------------

Message: 3
Date: Mon, 19 Sep 2005 11:29:45 -0400
From: Jim Hyslop <address@hidden>
Subject: Re: [task #4633] GPG-Signed Commits
To: Derek Price <address@hidden>
Cc: "Mark D. Baushke" <address@hidden>, address@hidden,
        address@hidden, address@hidden
Message-ID: <address@hidden>
Content-Type: text/plain; charset=ISO-8859-1; format=flowed

Derek Price wrote:
> One more thought on planning this feature, this is important enough to
> go into the stable release series, I think, but we are awfully close
to
> being able to bless feature as stable anyhow.
> 
> Would there be any objections to GPG-signed commits going into stable
as
> things stand?
> 
> Would there be any objections to 1.12.x being blessed as stable after
> adding GPG-signed commits, importing an updated diffutils, possibly
> completing the commitid stuff, and maybe an RC release or two?

Since security measures usually improve (or are completely disproved) 
with wide-spread review, I'd be disinclined to add it into the current 
stable release without at least _some_ field trials to make sure the 
approach is correct and bug-free.

I'd feel better with the second approach - add it to 1.12.x, with the 
other changes, produce as many RC releases as are required to get it 
right, then (hallelujah!) declare 1.12 released.

-- 
Jim





------------------------------

Message: 4
Date: Mon, 19 Sep 2005 10:09:03 -0700
From: "Mark D. Baushke" <address@hidden>
Subject: Re: [task #4633] GPG-Signed Commits 
To: Derek Price <address@hidden>
Cc: address@hidden, address@hidden, address@hidden
Message-ID: <address@hidden>
Content-Type: text/plain; charset=us-ascii

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Derek Price <address@hidden> writes:

> One more thought on planning this feature, this is
> important enough to go into the stable release series, I
> think, but we are awfully close to being able to bless
> feature as stable anyhow.

Hmmm... I think we are fairly close, but I suspect the
gpg-signed feature may take a lot of work to get correct...

> Would there be any objections to GPG-signed commits going
> into stable as things stand?

I tend to agree with Jim Hyslop's comments regarding getting
some field trials in place and ensuring that it is bug-free
before considering back-porting it to 1.11.x...

I think it would be faster to push 1.12.x into being a
'stable' release...

> Would there be any objections to 1.12.x being blessed as
> stable after adding GPG-signed commits, importing an
> updated diffutils, possibly completing the commitid stuff,
> and maybe an RC release or two?

I have no real objection. I think it is reasonable to list
the stuff that folks have submitted to the lists or the old
www.cvshome.org issuetracker that may wish to be considered
for 1.12.x before it becomes 'stable'.

The following may not be a complete 'wish list', so if I
have missed anything, feel free to add here for
consideration.

  - 'cvs import' goes through a pre-import trigger (be it a
    new trigger or be it a combination of commitinfo and
    taginfo)
    https://ccvs.cvshome.org/issues/show_bug.cgi?id=157

  - Updated diffutils.

  - The Frank Hemer <address@hidden> newtags feature.

  - A change to 'cvs add', either it goes through a
    commitinfo trigger for new directories (it also needs to
    get a CVS/Template populated when it is created which
    seems to be a minor bug), or going with Greg A Woods
    <address@hidden> suggestion to defer the connection to
    the server until the 'cvs commit' has been issued (ie,
    make cvs add client/server semantics like cvs rm
    symmetric). The thread is here:
    http://lists.gnu.org/archive/html/info-cvs/2005-02/msg00014.html
    There is a long-standing issue here:
    https://ccvs.cvshome.org/issues/show_bug.cgi?id=2

  - GPG-signed feature.

    + This means some way to have the server manage GPG key
      material in some kind of a cvs server 'web-of-trust'.

    + There is a need to deal with expired keys and revoked
      keys as well as with successor keys.

    + There may need to be a new way for a client to obtain
      the public keys related to all files in the repository
      as a part of a 'verify' operation... would this imply
      running an hkp: or ldap: server on the cvs host, or
      would cvs have additional client/server protocol put
      in place to handle this?

    + Client-side user-requested 'diff' replacement (to
      allow validation of all versions related to a 'diff'
      as well as to provide a replacement of 'diff' with
      graphical or special-purpose comparisons (e.g., binary
      difference engines or xml difference engines).

    + Client-side user-requested 'diff3' replacement (to
      allow validation of all versiosn related to a 'merge'
      operation as well as to provide a replacement 'diff3'
      with a graphical or special-puposes merge operation
      (e.g., allow a special-purpose program to merge
      graphics or other 'binary' files).

  - cvs output should be able to support xml to allow
    projects like cvslib and other cvs clients to more
    effectively communicate with the server.
    /issues/show_bug.cgi?id=157

  - Per recent discussions on Unicode, the possibility of a
    new feature that is orthogonal to keyword expansion that
    controls if line endings conversion should take place or
    not. (This may be desirable for GPG-signed form in any
    case.)

  - Desire the ability to deal with execute bits between
    'cvs rm' and a new 'cvs add' of the same filename.
    Possibly a new admin command to change the execute bits
    on the repository file?
    https://ccvs.cvshome.org/issues/show_bug.cgi?id=115

  - Some folks desire a way for 'cvs import' and possibly
    'cvs add' to have a method to determine keyword
    substitution for new files
    https://ccvs.cvshome.org/issues/show_bug.cgi?id=1

  - Some folks desire an option to 'cvs update' to make the
    time of the updated file be the commit timestamp rather
    than the update timestamp.
    https://ccvs.cvshome.org/issues/show_bug.cgi?id=150

  - As a robustness, when LockDir is not being used, it may
    be desirable to prohibit directories that look like cvs
    internal lock files.
    https://ccvs.cvshome.org/issues/show_bug.cgi?id=183

CVSNT 'features' that I have heard mentioned might be
'useful' to port back to CVS are:

  - the 'mergepoint' feature 

  - the 'LockServer' (faster and more granular locks) feature

  - the 'access control lists' feature

  - pluggable server-side diff programs (I do not believe
    that pluggable server-side diff programs are going to be
    useful if the GPG-signed feature is enabled).

I think that about covers the features I recall being
requested... I am not saying that they all need to go into
1.12.x before it becomes stable, just that they should be
addressed in some way to set expectations before 1.12.x goes
to the first release candidate.

        Thanks,
        -- Mark
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.3 (FreeBSD)

iD8DBQFDLvCvCg7APGsDnFERApPtAJ4sAVsdSibBU8inmDBp2DewKWESFACgoyGE
hArL0rEwsjUfWc0mdP/mK2Y=
=x+lR
-----END PGP SIGNATURE-----




------------------------------

Message: 5
Date: Mon, 19 Sep 2005 18:00:15 +0000
From: Derek Robert Price <address@hidden>
Subject: [task #4687] Import recent diffutils.
To: Derek Robert Price <address@hidden>, address@hidden
Message-ID: <address@hidden>
Content-Type: text/plain;charset=UTF-8


URL:
  <http://savannah.nongnu.org/task/?func=detailitem&item_id=4687>

                 Summary: Import recent diffutils.
                 Project: Concurrent Versions System
            Submitted by: dprice
            Submitted on: Mon 09/19/05 at 18:00
         Should Start On: Mon 09/19/05 at 00:00
   Should be Finished on: Mon 09/19/05 at 00:00
                Category: None
                Priority: 1 - Later
                  Status: None
                 Privacy: Public
             Assigned to: None
        Percent Complete: 0%
             Open/Closed: Open
                  Effort: 0.00

    _______________________________________________________

Details:

This should get us UTF-8 Support, at least for `cvs diff
--side-by-side'.






    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/task/?func=detailitem&item_id=4687>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.nongnu.org/





------------------------------

Message: 6
Date: Mon, 19 Sep 2005 17:58:44 +0000
From: Derek Robert Price <address@hidden>
Subject: [task #4686] Bless 1.12.x stable.
To: Derek Robert Price <address@hidden>, address@hidden
Message-ID: <address@hidden>
Content-Type: text/plain;charset=UTF-8


URL:
  <http://savannah.nongnu.org/task/?func=detailitem&item_id=4686>

                 Summary: Bless 1.12.x stable.
                 Project: Concurrent Versions System
            Submitted by: dprice
            Submitted on: Mon 09/19/05 at 17:58
         Should Start On: Mon 09/19/05 at 00:00
   Should be Finished on: Mon 09/19/05 at 00:00
                Category: None
                Priority: 5 - Normal
                  Status: None
                 Privacy: Public
             Assigned to: None
        Percent Complete: 0%
             Open/Closed: Open
                  Effort: 0.00

    _______________________________________________________

Details:

Bless feature series (1.12.x) as stable.






    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/task/?func=detailitem&item_id=4686>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.nongnu.org/





------------------------------

Message: 7
Date: Mon, 19 Sep 2005 18:09:14 +0000
From: Derek Robert Price <address@hidden>
Subject: [patch #4441] Add importinfo trigger,  or cause import to run
        commitinfo/loginfo combination as does  commit.
To: address@hidden, Derek Robert Price <address@hidden>,
        address@hidden
Message-ID: <address@hidden>
Content-Type: text/plain;charset=UTF-8


URL:
  <http://savannah.nongnu.org/patch/?func=detailitem&item_id=4441>

                 Summary: Add importinfo trigger, or cause import to run
commitinfo/loginfo combination as does commit.
                 Project: Concurrent Versions System
            Submitted by: dprice
            Submitted on: Mon 09/19/05 at 18:09
                Category: None
                Priority: 5 - Normal
                  Status: None
                 Privacy: Public
             Assigned to: None
        Originator Email: 
             Open/Closed: Open
           Fixed Release: None
   Fixed Feature Release: None

    _______________________________________________________

Details:

Was https://ccvs.cvshome.org/issues/show_bug.cgi?id=157:

We have a big problem with people accidentally 
running the "import" command (unfortunately, 
WinCVS puts "checkout" and "import" right next to 
each other in its menu, and people end up 
clicking the wrong menu item from time to time.

What I've done locally is to modify the import 
command to look for a file 
named "CVSROOT/importallow" and verify that the 
current user is in this list (effectively 
restricting import permissions to a few dozen 
users).  If the "importallow" file is not 
present, then anybody is presumed to be allowed 
to perform imports.

I'd like to submit this patch for inclusion in 
mainline CVS.  Does this sound like a useful 
inclusion?



------- Additional comments from Joshua Davies Thu Feb 19 10:06:17 -0700
2004
-------

Created an attachment (id=67)
"importallow" patch



------- Additional comments from Joshua Davies Thu Feb 19 10:07:28 -0700
2004
-------

Sorry it took so long to include this patch; I ran into some 
technical difficulties.  This was tested against source release 
1.12.5.  This patch has been running in our site for over six months 
now and has helped us tremendously.



------- Additional comments from Derek Price Mon Mar 15 16:44:58 -0700
2004
-------

I looked over this patch and have several comments:

1.  It needs src/sanity.sh test cases and documentation in
doc/cvs.texinfo.  Please read the HACKING file in the top level of the
CVS source distribution for more if you don't understand this.

2.  I think the comparison of the current user name to the readers,
writers, or importallowed file should be consolidated into a single
function that accepts the file the current username is being
authenticated against as an argument.  This is way too much
duplication of code.

3.  The default comment in the importallowed file created by
mkmodules.c should be more complete.  Plese see the comments in the
readers, writers, and *info file for examples.

4.  How about renaming the file "importers" for consistency with the
"readers" and "writers" files?

5.  The "not authorized to use this command" error message needs at
least a comment in the code about it being easy to miss the "checkout"
menu option and hit "import" with WinCVS.  In the context of a command
line client this error message is much too cryptic.

I am also going to send a message off to address@hidden in hopes of
starting a more general discussion about this patch.  If you can
update your patch with regards to the above criticisms and any further
valid criticisms and the bug-cvs discussion turns out ok, then I will
commit this patch.



------- Additional comments from Derek Price Mon Mar 15 16:45:40 -0700
2004
-------

Incidentally, thanks for the patch submission!



------- Additional comments from Derek Price Mon Mar 15 16:48:25 -0700
2004
-------

General questions this patch raises for me:

1.  This makes me itch for a more general permissions scheme, but
nothing comes instantly to mind.  My current inclination is to allow
this patch (assuming my earlier criticisms are answered), but I would
love to hear discussion of a more general solution first.



------- Additional comments from Derek Price Mon Mar 15 16:52:33 -0700
2004
-------

Adding myself to the CC list.



------- Additional comments from Derek Price Mon Mar 15 18:01:47 -0700
2004
-------

Mark D. Baushke <address@hidden> writes:

I would rather see a CVSROOT/importinfo file with script that get run to
determine if the files being imported meet with the cvs administrator
critera for new imports.

Such a script could use something like cvs_acls to determine that a
given user was not allowed to do the import as well as determining that
the files being imported were not acceptable for other criteria.

One of the biggest things I'd like to see such a script be able to
handle is to disallow vendor and version tags that do not match the
criteria provided by the administrator. Right now taginfo is fairly
useless if someone can easily wipe-out an existing tag thru use of
a 'cvs import'...

Having the special-purpose CVSROOT/importers seems less flexible to me.

For that matter, I think it might be useful to have trigger scripts for
'cvs checkout' and 'cvs update' to allow a cvs administrator to provide
a finer grained policy on what users may do with files in a repository.

        -- Mark



------- Additional comments from Derek Price Mon Mar 15 18:05:05 -0700
2004
-------

This is exactly the sort of thing I was fishing for.  An importinfo
file should be fairly easy to implement now with the new
format_cmdline() hooks.



------- Additional comments from Ingolf Steinbach Tue Mar 16 05:16:02
-0700
2004 -------

Derek wrote on 2004-03-15: 
> 5.  The "not authorized to use this command" error message needs at 
> least a comment in the code about it being easy to miss the "checkout"

> menu option and hit "import" with WinCVS.  In the context of a command

> line client this error message is much too cryptic. 
 
I am not sure if comments in the cvs code should deal with mis-usage 
and/or pitfalls with certain versions of certain external tools. 
IMO, an improved error message like "you are not authorized to use the 
'import' command" (again without a reference to WinCVS or something like

that) would be better. 
 
Kind regards 
    Ingolf 



------- Additional comments from Derek Price Tue Mar 16 08:39:26 -0700
2004
-------

Larry Jones <address@hidden> writes:

>Mark D. Baushke writes:
>
>>> 
>>> I would rather see a CVSROOT/importinfo file with script that get
run to
>>> determine if the files being imported meet with the cvs
administrator
>>> critera for new imports.
>
>I'll second that, with the caveat that I would still prefer to extend
>commitinfo rather than creating a new administrative file.



------- Additional comments from Derek Price Tue Mar 16 08:41:50 -0700
2004
-------

Extending commitinfo should be even easier to add with the new API.  A
single new field type containing the commit type would need to be
added, and the current commitinfo call placed in a function accessible
to both commit.c & import.c.



------- Additional comments from Derek Price Tue Mar 16 09:14:21 -0700
2004
-------

Mark D. Baushke Writes:

Extending the commitinfo would be acceptable to me provided that access
to both the vendor and version tags to be used for the import were also
somehow made available to make the decision to allow the import to
happen or not.



------- Additional comments from Derek Price Tue Mar 16 09:31:22 -0700
2004
-------

Good point again, Mark.  It should be useful and easy enough to extend
the current commitinfo hook to include a format string for the
destination branch name anyhow, and this could be reused for import's
vendor branch.  Even including the old revision for each file would be
easy.  Destination revision would be harder - it currently isn't set
for use by loginfo until it is retrieved after the actual commit.

As for the _list_ of release tags (don't forget it is a list), this
could be done, but would be a little more compilcated.  Perhaps if it
were simply left empty for calls to commit and filled in for calls to
import, that would be sufficient.  Using some characters forbidden in
tags, users could specify something like the following so that their
scripts could handle the double lists:

ALL $CVSROOT/CVSROOT/myscript %r %p $T %{t} -- %{sV}



------- Additional comments from Mark D. Baushke Tue Mar 16 09:45:39
-0700
2004 -------

Derek Robert Price <address@hidden> writes:

> Good point again, Mark. It should be useful and easy
> enough to extend the current commitinfo hook to include a
> format string for the destination branch name anyhow, and
> this could be reused for import's vendor branch. Even
> including the old revision for each file would be easy.
> Destination revision would be harder - it currently isn't
> set for use by loginfo until it is retrieved after the
> actual commit.

Agreed. The destination revision is a harder problem.
However, it might be useful to have another flag that could
be passed into the script to indicate that the operation is
either commit or import.

Would the commitinfo script be called once for the entire
tree as loginfo is called at present or once for each
directory in the import tree? This has implications for how
the contrib/commit_prep.in and contrib/log_accum examples
presently operate.

It might also be useful for the 'cvs add' and 'cvs import'
commands to share logic with 'cvs commit' as to validation
that the creation of a new directory in the repository.

> As for the _list_ of release tags (don't forget it is a
> list), this could be done, but would be a little more
> compilcated. Perhaps if it were simply left empty for
> calls to commit and filled in for calls to import, that
> would be sufficient. Using some characters forbidden in
> tags, users could specify something like the following so
> that their scripts could handle the double lists:
>
> ALL $CVSROOT/CVSROOT/myscript %r %p $T %{t} -- %{sV}

I do not understand the $T in the above line. Where is it
set?

> I've annotated Issuezilla with our last few comments again.

For a 'cvs import' I would be under the impression that
there are two active tags in the operation: 1) the vendor
tag, 2) the version tag. It is still not clear to me if you
intend to integrate commitinfo with the taginfo operation by
calling the taginfo script during import or not...



------- Additional comments from Derek Price Tue Mar 16 10:17:51 -0700
2004
-------

> Would the commitinfo script be called once for the entire
> tree as loginfo is called at present or once for each
> directory in the import tree? This has implications for how
> the contrib/commit_prep.in and contrib/log_accum examples
> presently operate.

Good question.  I would have said that it should be called for each
directory if you hadn't mentioned that loginfo wasn't being called
separately.  Perhaps it should be kept in sync?  As long as
commit_prep is called only once, I think log_accum will still work
properly.  I'm not sure I understand all the issues involved, but you
are probably right that a once-per-directory/once-per-commit
combination would break the examples.

> It might also be useful for the 'cvs add' and 'cvs import'
> commands to share logic with 'cvs commit' as to validation
> that the creation of a new directory in the repository.

Hrm.  Yes, probably, but beyond the scope of my voting for acceptance
of this patch, I think.

> >ALL $CVSROOT/CVSROOT/myscript %r %p $T %{t} -- %{sV}
>
> I do not understand the $T in the above line. Where is it
> set?

Er, it should have been %T, and I meant it to be the destination
branch/vendor branch.  Maybe %B would be better?  Anyhow, taginfo is
using %t for the name of the tag being created and %b as a
branch/non-branch flag, and I was trying to remain somewhat
consistent.  I thought %t/%T was somewhat consistent with the %{Vv}
being used for old revision number/new revision number model being
used for loginfo & taginfo.

And %T isn't being set yet.  I was just suggesting it.

> For a 'cvs import' I would be under the impression that
> there are two active tags in the operation: 1) the vendor
> tag, 2) the version tag. 

Actually, there can be more than one release tag (what you are calling
a vesion tag):

Usage: cvs import [-d] [-k subst] [-I ign] [-m msg] [-b branch]
    [-W spec] repository vendor-tag release-tags...

I don't know how often people use that functionality, but as long as
it is enabled, it would need to be accounted for.

> It is still not clear to me if you
> intend to integrate commitinfo with the taginfo operation by
> calling the taginfo script during import or not...

Ouch.  I hadn't thought of that.  It's a thought, but I was still
thinking of leaving that to the commitinfo script...  if a user really
wanted, it shouldn't be too hard to call their own taginfo script from
their commitinfo script on an import, if that's what they wanted.  I
think I will vote for leaving taginfo out of the import until it looks
like it is really necessary, or at least useful, to separate the
functionality.



------- Additional comments from Derek Price Tue Mar 16 14:05:56 -0700
2004
-------

Larry Jones writes:

>Ouch.  I hadn't thought of that.  It's a thought, but I was still
>> thinking of leaving that to the commitinfo script...  if a user
really
>> wanted, it shouldn't be too hard to call their own taginfo script
from
>> their commitinfo script on an import.  I think I would vote for
leaving
>> taginfo out of the import until it looks like it is really
necessary, or
>> at least useful, to separate the functionality.

It seems to me that import is basically a shorthand for commit and tag
operations and thus should use the existing mechanisms.  That way users
don't have to put the same stuff in two different places (which always
leads to synchronization problems).



------- Additional comments from Joshua Davies Thu Mar 18 10:14:25 -0700
2004
-------

Wow - I've sparked quite a debate, here.  I'd be more than happy to 
apply the five changes suggested by Derek Price on 3/15, but it looks 
like the consensus is that a new "importallow" script isn't the most 
flexible way to handle this issue.  It seems like the consensus is 
that import should invoke "commitinfo" - I'll have to do some 
research to understand how the "commitinfo" script is invoked to see 
how it can be extended.

Minor comment from an FNG, though - if, as Mark Baushke suggests, 
commitinfo should be invoked as part of add, commit & import... 
should the name of the file be changed?  (With a "new" file name 
searched for and "commitinfo" called as a fallback for migration - 
sort of like the XF86Config-4 migration)?  Just a thought.



------- Additional comments from Derek Price Thu Mar 18 11:00:30 -0700
2004
-------

I wouldn't think a filename change is necessary, per se.  Add of a
directory and import are both performing commits, basically.

Neither would I consider the name change a big deal if you want to
change it.  I suppose it might be a trifle less confusing to newbies
to see a more general name not so directly associated with commit.



------- Additional comments from Joshua Davies Sat Mar 27 09:40:57 -0700
2004
-------

Ok, I've made some progress on this - and dug much deeper into the CVS
source code than I ever thought I would.  One observation I wouldn't
mind getting some feedback on - if the "import" command invokes the
"commitinfo" script, obviously none of the modules will ever match, so
if you want to control import behavior, you'd need to add a script
under DEFAULT or ALL.

Would it make sense to add another keyword, IMPORT, to be invoked on
import?  So you're commitinfo script might look like this:

module1  $CVSROOT/CVSROOT/sanity-check.sh %p %r %{s}
DEFAULT  $CVSROOT/CVSROOT/verify.sh %p %r %{s}
IMPORT   $CVSROOT/CVSROOT/check-user.sh %r %p $T %{t} -- %{sV}
ALL      xxx



------- Additional comments from Derek Price Sat Mar 27 11:36:09 -0700
2004
-------

Do you have a good reason for wanting to differentiate the import from
the creation of other directories and files?  I can't think of a good
reason.

If there is a good reason, what advantage does the IMPORT flag have
over passing a string that resolves to import/checkout/add into the
called script as an argument if requested by the user?  I think I see
disadvantages given that putting this functionality into commitinfo
was a way to consolidate the new functionality where it made sense.



------- Additional comments from Joshua Davies Sun Mar 28 08:21:06 -0700
2004
-------

Well, remember that my original use-case was to restrict the
import command entirely - so I'm going to end up writing a "DEFAULT"
script like this:

#!/bin/sh

if [ "$1" == "import" ]
then
        # restrict access
else
        # do some style-checking and strip off dos-style newlines, etc.
fi

Let me do this - I'm almost done with the change to make the import
command invoke the commitinfo scriptset (without the special "IMPORT"
keyword); I'll submit another patch, let you look at it, and if you
think I should add the IMPORT keyword to the commitinfo script, I'll
do so and submit another patch.



------- Additional comments from Derek Price Sun Mar 28 09:47:15 -0700
2004
-------

Sounds good.



------- Additional comments from Derek Price Mon Jun 7 06:20:52 -0700
2004
-------

*** Issue 184 has been marked as a duplicate of this issue. ***



------- Additional comments from Joshua Davies Wed Jul 28 13:10:32 -0700
2004
-------

In case you're curious, I haven't given up on this... it's just a lot
trickier 
than I ever really anticipated.  I've finally gotten some time to come
back
and 
look at this, and my original use case (trigger the "commitinfo" script
as
part 
of each "import_descend" action and allow the commitinfo default script
to 
accept or reject an import) is working fine.  Of course, CVS has gone
through
3 
minor revisions while I was messing around with it ;), but I'm current
with 
1.12.9 at the moment.

Adding the branch & version info is a bit trickier, though, and this
seems to

be what issue #184 wants (we've ad-hoc'ed this at our site, so this
would 
probably be beneficial to us as well).  I'm not entirely sure how to
tackle 
this, since the branch name could vary from file to file (for all
practical 
purposes, it never does, but CVS *does* allows this), so it seems like
each 
file in the %{s} list on comminfo should be followed by the branch name,
the

old revision, and the new revision (so that the new format string would
be %
{sbvV} or something like that).



    _______________________________________________________

Carbon-Copy List:

CC Address                          | Comment
------------------------------------+-----------------------------
address@hidden               | Reporter of issue from
cvshome.org.



    _______________________________________________________

File Attachments:


-------------------------------------------------------
Date: Mon 09/19/05 at 18:09  Name: importallow.diff  Size: 4.21KB   By:
dprice
Joshua Davies's original patch submission.
<http://savannah.nongnu.org/patch/download.php?item_id=4441&item_file_id
=5211>

    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/patch/?func=detailitem&item_id=4441>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.nongnu.org/





------------------------------

_______________________________________________
Bug-cvs mailing list
address@hidden
http://lists.nongnu.org/mailman/listinfo/bug-cvs

End of Bug-cvs Digest, Vol 20, Issue 7
**************************************

reply via email to

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