git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Ed Santiago <santiago@redhat.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH] git-cvsimport: strip question-mark characters in tags
Date: Wed, 14 Apr 2010 09:44:10 -0600	[thread overview]
Message-ID: <20100414154409.GA28268@f.santiago.vpn.redhat.com> (raw)
In-Reply-To: <7vfx2yqf9s.fsf@alter.siamese.dyndns.org>

On Wed, Apr 14, 2010 at 07:29:03AM -0700, Junio C Hamano wrote:
> 
> I agree that people may want to optionally replace them to avoid mapping
> two originally different tags into the same one.

I considered that but decided that it was beyond the scope of
what I wanted to tackle.  My intent was to suggest an incremental
fix, not a revolutionary one.  I also wished to remove a misleading
diagnostic[1] and provide a way for others to do the same as needed.

 [1] The error I saw was "Cannot create tag mumblefoo{?dist}: Illegal seek",
     where "Illegal seek" is due to an uninitialized/invalid $!

A proper solution would, I believe, be uncomfortably complex.
Some of the approaches I considered are:

  * command-line option '--tag-replace <from-char>=<to>', eg '?=-'
  * replacing a (hardcoded) list of chars with %xx hex codes
  * option a la -A, with a file listing <oldtag>=<newtag>
  * like above, but the file lists perl expressions such as s/\?/./g

I used git-cvsimport because I've used tailor[2] before and
dreaded the thought of running it again.  I love how easy
git-cvsimport was to use!  I see it as a great tool for
quick jobs, for proof-of-concept situations: it has a low
cost of entry, works well with a remote CVS repo, and was
easy to debug.  But if it were to become as configurable -- and
as complex -- as tailor, I think it might lose much of its value.

 [2] http://wiki.darcs.net/RelatedSoftware/Tailor

> Does your regexp chain check other invalid refname sequences, such as ".name",
> "na..me", etc.?

Eek, no.  Again, this was a purely incremental fix, an addition
to an existing (simple) check.  Duplicating the functionality
of git-check-ref-format would be a terrible idea.  But if there's
already a perlified library for this, or a portable hook into
git itself (I haven't looked), it might be wise to have
git-cvsimport use that to check/convert tags.  At the very
least it would give a much better error to the end user.

> Sign-off?

My apologies.  I'm new to this.

How would you like me to proceed?  I'm reluctant to code a
full general solution to the tag-validity problem, but am
happy to rework my changes into something better.

Thanks for your feedback,
Ed
-- 
Ed Santiago             Software Engineer             santiago@redhat.com

  reply	other threads:[~2010-04-14 15:44 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-04-14 13:38 [PATCH] git-cvsimport: strip question-mark characters in tags Ed Santiago
2010-04-14 14:29 ` Junio C Hamano
2010-04-14 15:44   ` Ed Santiago [this message]
2010-04-14 19:27     ` Junio C Hamano
2010-04-14 20:44 ` Andreas Schwab
2010-04-14 21:42   ` Ed Santiago
2010-04-15  1:39     ` Johan Herland

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20100414154409.GA28268@f.santiago.vpn.redhat.com \
    --to=santiago@redhat.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).