- Status Closed
- Percent Complete
- Task Type Patches
- Category Infrastructure → Build environment
-
Assigned To
MikeS - Operating System All players
- Severity Low
- Priority Very Low
- Reported Version Release 3.10
- Due in Version Undecided
-
Due Date
Undecided
- Votes
- Private
FS#12554 - Strip SVN $Id$ field from all files
Not many details. If we're not going to use it, then strip the $Id$ field that used to hold file properties from everything, if we care to.
Closed by MikeS
2012-01-30 23:03
Reason for closing: Later
Additional comments about closing: Warning: Undefined array key "typography" in /home/rockbox/flyspray/plugins/dokuwiki/inc/parserutils.php on line 371 Warning: Undefined array key "camelcase" in /home/rockbox/flyspray/plugins/dokuwiki/inc/parserutils.php on line 407
2012-01-30 23:03
Reason for closing: Later
Additional comments about closing: Warning: Undefined array key "typography" in /home/rockbox/flyspray/plugins/dokuwiki/inc/parserutils.php on line 371 Warning: Undefined array key "camelcase" in /home/rockbox/flyspray/plugins/dokuwiki/inc/parserutils.php on line 407
No point for now.
Loading...
Available keyboard shortcuts
- Alt + ⇧ Shift + l Login Dialog / Logout
- Alt + ⇧ Shift + a Add new task
- Alt + ⇧ Shift + m My searches
- Alt + ⇧ Shift + t focus taskid search
Tasklist
- o open selected task
- j move cursor down
- k move cursor up
Task Details
- n Next task
- p Previous task
- Alt + ⇧ Shift + e ↵ Enter Edit this task
- Alt + ⇧ Shift + w watch task
- Alt + ⇧ Shift + y Close Task
Task Editing
- Alt + ⇧ Shift + s save task
It would be possible to have git put the sha1 there, or to use custom clean and smudge filters to put other info there. However, that's only useful when the file has been removed from version control, and it's not possible to obtain that information from git. I don't think there's a need to support that, so I agree with the removal of $Id$.
Two things:
1) There are some files that use things other than $Id$ - we should remove all the expansions.
2) When I looked at this before it looked like there were some files whose $Id$ lines were coming from upstream files and may have been pre-expanded - if we can sanely identify which these are it might be nice to leave them alone since it reduces the diff from upstream :)
@Torne:
1) So far I haven't run into them and so don't know what to search for, unless using "$<some max num chars>$" is enough to catch them
2) I didn't catch that here. I strictly searched "$Id$" replacement for now.
This is what I get playing loose with the matching. See anything of interest?
Here's what I found. I included the filenames, because they can help decide what to do and they can help accurately apply an automated edit. The list may be complete, but some lines need special handling. I'll paste some of them here:
apps/scrobbler.c:#define SCROBBLER_REVISION " $Revision$"
apps/plugins/lua/action_helper.pl:printf "– It is automatically generated of action.h %s\n", '$Revision$';
apps/plugins/lua/button_helper.pl:$svnrev = '$Revision$';
apps/plugins/lua/rocklib_aux.pl:my $svnrev = '$Revision$';
rbutil/rbutilqt/Info.plist: <string>SVN $Revision$ (1.2.11)</string>
rbutil/rbutilqt/version.h:#define PUREVERSION "SVN $Revision$"
tools/configure:scriptver=`echo '$Revision$' | sed -e 's:\\$::g' -e 's/Revision: //'`
tools/mp3info.pm:($REVISION) = ' $Revision$ ' =~ /\$Revision:\s+([^\s]+)/;
It should be possible to identify many files coming from upstream based on path. I'm not sure if unexpanded tags should be kept just to reduce the diff. However, tags that have been expanded using information from the upstream repository should probably be kept to help figure out what version the code was taken from.
Why do you have expanded $Id$ in many of yours? None of my files are currently as such. Here's the same overly "greedy" search I did previously with the filenames included this time. Compare ./rbutil/ipodpatcher/fat32format.c for instance.
I think you're confusing two files:
grep fat32format search2.txt search3.txt
search2.txt:bootloader/fat32format.c: * $Id: fat32format.c 30351 2011-08-25 19:58:47Z thomasjfox $
search2.txt:rbutil/ipodpatcher/fat32format.c: * $Id$
search3.txt:./rbutil/ipodpatcher/fat32format.c: * $Id$
bootloader/fat32format.c definitely has an expanded Id: http://git.rockbox.org/?p=rockbox.git;a=blob;f=bootloader/fat32format.c;h=540ee89b548739cd7cfd0a58507f68fd37953c0e;hb=HEAD
You may want to examine what's only in one of the two files, for example starting with diff <(sort < search2.txt) <(sed "s/^\.\///" search3.txt | sort)
Here are two more special cases:
manual/pdfdraftcopy.sty:\Filedate$Date: 2003/08/11 20:31:07 $
manual/pdfdraftcopy.sty:\Fileversion$Revision: 1.2 $
Argh, I used the wrong regex, not what I thought I did. Ok, there's nothing special about the ones that are expanded over the ones that aren't that I can see.
I guess they 're in the repository already that way and there's nothing more to that.
Fine, here's one that unexpands those. Anyway, just commit to having straggling "$Id$" or not IMHO. It's a heck of alot easier to leave it if it is wanted for later than to restore it all.
For example, in apps/plugins/lua/lapi.c the following comes from Lua, not Rockbox:
- $Id: lapi.c,v 2.55.1.5 2008/07/04 18:41:18 roberto Exp $
+ $Id$
I think the information could be useful if someone needs to backport fixes from Lua. It could be used to figure out what version of Lua the file originated from. I guess it's not useful for decreasing the diff from upstream, because it will be changed whenever the file is updated in Lua. I'm attaching a file containing parts of search2.txt which are in code from elsewhere, which doesn't have a Rockbox header. It could be used to avoid changing those parts, if desired.
Regarding the special cases I mentioned earlier, git $Id$ could be used instead of $Revision$ wherever expansion is needed. The only non-cosmetic issue I found is in rbutil/rbutilqt/. Currently, rbutilqt.cpp won't notice a version change because the string will always literally be "SVN $Revision$", so the user won't be asked to verify the settings. In configure, scriptver isn't used so it can just be deleted. There is no need to touch manual/pdfdraftcopy.sty. Should the git attributes go into .gitattributes in the directory where the file is or elsewhere?
Sure, we can hand prune some things where rockbox SVN isn't the one responsible for the expansion. However, some were added even though they weren't rockbox headers. The ones in libmpeg2 for mpegplayer I added myself.