273209 - Checksetup.pl-created skins/.cvsignore is annoying in cvs update

Status: RESOLVED FIXED

(Bugzilla :: Installation & Upgrading, defect)

Description

[Jouni Heikniemi]

I'm filing this as a new bug, as it's an easy way to make whatever opinions here
logged. Targeting & ccing original posse from bug 256208.

Now that we're creating skins/.cvsignore to include the "custom" directory, I
get a "? skins/.cvsignore" every time I run cvs up. This doesn't feel good
either. How much of an issue would checking that .cvsignore in be? How often do
people need to change it (from the default content of "custom")?

Comment 1

[timeless]

if you're going to make a .cvsignore file, then you should mention .cvsignore in it...

Comment 2

[Dave Miller [:justdave]]

Is there stuff inside the skins directory that IS in cvs?  If so, then by all
means, that .cvsignore should be checked in.  If skins is otherwise empty, then
just mention .cvsignore in the .cvsignore file as timeless suggests.

Comment 3

[Myk Melez [:myk] [@mykmelez]]

Installations are going to list installed third-party skins in skins/.cvsignore
so that CVS won't bother with them.  If we check in skins/.cvsignore, it's going
to claim that it's modified on "cvs update" and display changes on "cvs diff",
which we don't want, so instead of checking it in we should add it to the root
.cvsignore.

Comment 4

[Myk Melez [:myk] [@mykmelez]]

Attachment: patch v1: fixes problem

Here's a patch that adds .cvsignore to skins/.cvsignore so that the file
instructs CVS to ignore it.  I've also simplified the code so that it creates
skins/.cvsignore if it doesn't exist but doesn't otherwise modify it.

The downside to this simplification is that if "custom" ever gets accidentally
deleted from the file, it won't be replaced, and CVS commands will question its
existence.  The upside, besides the code being simpler and more obvious, is
that installation admins will have more control over the contents of the file,
and if for some reason they don't want custom to be ignored, we won't thwart
them and force CVS to ignore it anyway.

Since admins hacking the file will know what they're doing, and since the only
consequence of the downside is a "? skins/custom" as part of the output of some
CVS commands, the upside of this change wins.

Comment 5

[Myk Melez [:myk] [@mykmelez]]

This polish fix should make it into 2.20.

Comment 6

[Jouni Heikniemi]

Comment on attachment 179833 [details] [diff] [review]
patch v1: fixes problem

r=jouni, although this will only fix new installations. But I agree, let's not
mess with the cvsignore contents once it's been created (even though it might
be sensible to do it _once_ here).

Comment 7

[Myk Melez [:myk] [@mykmelez]]

Checking in checksetup.pl;
/cvsroot/mozilla/webtools/bugzilla/checksetup.pl,v  <--  checksetup.pl
new revision: 1.388; previous revision: 1.387
done