You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@spamassassin.apache.org by bu...@bugzilla.spamassassin.org on 2011/05/27 16:51:50 UTC
[Bug 6605] New: Suboptimal results when svn keywords aren't expanded
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6605
Bug #: 6605
Summary: Suboptimal results when svn keywords aren't expanded
Product: Spamassassin
Version: SVN Trunk (Latest Devel Version)
Platform: All
OS/Version: All
Status: NEW
Severity: trivial
Priority: P2
Component: spamassassin
AssignedTo: dev@spamassassin.apache.org
ReportedBy: Darxus@ChaosReigns.com
Classification: Unclassified
The Ubuntu daily builds stuff is not capable of expanding svn keywords, which
is quite clearly a bug in their software, but spamassassin could handle it
better.
(I expect to be the one to fix this.)
Symptoms:
The two instances of "updated" in this should be numbers:
X-Spam-Checker-Version: SpamAssassin 3.4.0-rupdated (updated) on
panic.chaosreigns.com
/etc/cron.daily/spamassassin:
Use of uninitialized value in concatenation (.) or string at /usr/bin/sa-update
line 23.
Example code lines as retrieved via svn, with svn keywords expanded:
$SUB_VERSION = (split(/\s+/,'$LastChangedDate: 2010-03-30 08:02:18 -0400 (Tue,
30 Mar 2010) $ updated by SVN'))[1];
('r' . qw{$LastChangedRevision: 929098 $ updated by SVN}[1]));
Same lines retrieved with Ubuntu build recipe, without svn keyword expansion:
$SUB_VERSION = (split(/\s+/,'$LastChangedDate$ updated by SVN'))[1];
('r' . qw{$LastChangedRevision$ updated by SVN}[1]));
In sa-update, the problem is $Id$:
my $VERSION = 'svn' . (split(/\s+/,
'$Id$'))[2];
It looks like the keywords used are defined in these files:
trunk$ grep -rl svn:keywords .
For reference, the bug against the Ubuntu building system:
https://bugs.launchpad.net/launchpad/+bug/780916
--
Configure bugmail: https://issues.apache.org/SpamAssassin/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
[Bug 6605] Suboptimal results when svn keywords aren't expanded
Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6605
Kevin A. McGrail <km...@pccc.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #4908|0 |1
is obsolete| |
--- Comment #8 from Kevin A. McGrail <km...@pccc.com> 2011-05-27 23:07:48 UTC ---
Created attachment 4910
--> https://issues.apache.org/SpamAssassin/attachment.cgi?id=4910
Minory Revised patch
--
Configure bugmail: https://issues.apache.org/SpamAssassin/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
[Bug 6605] Suboptimal results when svn keywords aren't expanded
Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6605
--- Comment #4 from Darxus <Da...@ChaosReigns.com> 2011-05-27 21:45:19 UTC ---
And I don't think this needs voting, just a commit (after testing), since I
don't have the access to do it myself.
--
Configure bugmail: https://issues.apache.org/SpamAssassin/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
[Bug 6605] Suboptimal results when svn keywords aren't expanded
Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6605
--- Comment #1 from Darxus <Da...@ChaosReigns.com> 2011-05-27 19:05:31 UTC ---
Created attachment 4908
--> https://issues.apache.org/SpamAssassin/attachment.cgi?id=4908
Patch, should fix mentioned symptoms, but NOT TESTED
There are lots of other svn keywords in the code. I'm thinking it's not worth
handling the rest of them.
--
Configure bugmail: https://issues.apache.org/SpamAssassin/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
[Bug 6605] Suboptimal results when svn keywords aren't expanded
Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6605
--- Comment #6 from Darxus <Da...@ChaosReigns.com> 2011-05-27 22:18:58 UTC ---
You're missing handling of the keyword expansion that is the very reason for
the patch. Apparently svn doesn't provide it at all (for patching) before
v1.7.x, which is still only available from svn's svn repo (unreleased).
Terrible.
Two options:
1) Upgrade to svn v1.7.x and apply with svn patch.
2) Manually collapse the keywords in the files before applying the patch:
Change
$LastChangedDate: 2010-03-30 08:02:18 -0400 (Tue, 30 Mar 2010) $
to
$LastChangedDate$
$LastChangedRevision: 929098 $
$LastChangedRevision$
$Id: sa-update.raw 917659 2010-03-01 19:24:41Z mmartinec $
$Id$
This problem with svn is discussed here:
http://mail-archives.apache.org/mod_mbox/subversion-users/201002.mbox/%3C20100205190549.GF16320@jack.stsp.name%3E
svn diff already handles this stuff.
--
Configure bugmail: https://issues.apache.org/SpamAssassin/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
[Bug 6605] [review] Suboptimal results when svn keywords aren't
expanded
Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6605
Kevin A. McGrail <km...@pccc.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Summary|Suboptimal results when svn |[review] Suboptimal results
|keywords aren't expanded |when svn keywords aren't
| |expanded
--- Comment #12 from Kevin A. McGrail <km...@pccc.com> 2011-05-28 01:15:13 UTC ---
Committed to trunk
New Revision: 1128544
URL: http://svn.apache.org/viewvc?rev=1128544&view=rev
Log:
SVN Keyword handling improvement bug 6605
Modified:
spamassassin/trunk/lib/Mail/SpamAssassin.pm
spamassassin/trunk/sa-update.raw
I had to rm my entire trunk svn and re-checkout but upon make, I now see :
./sa-update --version
sa-update version svn1128544
and
./spamassassin --version
SpamAssassin version 3.4.0-r1128544
Please let me know if the daily builds work now for you hence changing to
[review] statuso.
--
Configure bugmail: https://issues.apache.org/SpamAssassin/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
[Bug 6605] Suboptimal results when svn keywords aren't expanded
Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6605
--- Comment #7 from Darxus <Da...@ChaosReigns.com> 2011-05-27 22:55:41 UTC ---
I guess I should explain svn keywords (which I had to figure out to create this
patch).
"$Id$", for example, is a svn keyword. A kind of variable specific to svn.
It's stored in svn ad "$Id$", but when you download it from svn with the svn
client, it expands to something like:
"$Id: sa-update.raw 1028810 2010-10-29 15:43:10Z mmartinec $"
And after I made my modifications, when svn diff created the patch for me, it
collapsed the keywords back to, for example, "$Id$".
So you're trying to apply my patch with collapsed keywords to your copy with
expanded keywords.
So you just need to collapse my keywords by stripping out the extra stuff, so
that my patch will apply.
...And then you need them expanded for testing. Which I don't know how to do
without committing to svn.
svn seems to be seriously lacking here.
--
Configure bugmail: https://issues.apache.org/SpamAssassin/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
[Bug 6605] Suboptimal results when svn keywords aren't expanded
Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6605
--- Comment #9 from Kevin A. McGrail <km...@pccc.com> 2011-05-27 23:13:01 UTC ---
Definitely a chicken before the egg issue if I understand things correctly.
First, I can't upgrade a production server to use a non-released SVN
unfortunately. So that's a no-go.
Second, I need to also understand more about how this might affect ASF stuff
because I have no idea what version of SVN jenkins uses and people/minotar uses
1.6.16. And it's a multiple hour issue to upgrade SVN for the ASF project due
to replication in the EU and US.
Third, let's ignore the debian issue for the moment and focus on the $Id$ for
example,
When I make the change to $Id$ in sa-update-raw and run svn diff on my devel
machine, svn outputs NOTHING as if the file hasn't even changed.
On Minotaur, I see this SAME behavior as well so I can't even commit the change
because SVN doesn't think a change occurred.
my $VERSION = 'svn' . (split(/\s+/,
'$Id: sa-update.raw 1028810 2010-10-29 15:43:10Z mmartinec $'))[2];
So I'm guessing that is expected behavior because svn is sort of hiding this
auto variable handling i.e. svn keywords.
Fourth, when I change to $Id$ and $LastChangeDate$, etc., I see the
uninitialized errors you are talking about with make test.
Fifth, the patch doesn't compile due to a missing closing parens in this logic.
I also changed some small things for consistency and attached a new version.
+ push(@EXTRA_VERSION,
+ ('r' . 'unknown');
+ }
OK, so with that said, the conclusion is that I have made a patch and the
various unitiliazed errors are gone. The trunk passes make test.
By my theory and I think your explanation of svn keywords, once I commit this
and run svn update, it should auto expand again... and then if I make, where
versions look like this now:
sa-update version svnunknown
SpamAssassin version 3.4.0-rsvnunknown
They'll change to something like svn<revision>.
Are we on the same page and we are ready to try a commit as described?
--
Configure bugmail: https://issues.apache.org/SpamAssassin/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
[Bug 6605] Suboptimal results when svn keywords aren't expanded
Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6605
--- Comment #11 from Darxus <Da...@ChaosReigns.com> 2011-05-27 23:26:30 UTC ---
(In reply to comment #9)
> First, I can't upgrade a production server to use a non-released SVN
> unfortunately. So that's a no-go.
I wasn't talking about upgrading svn on the server side, I was talking about on
the client side - the one where you're running svn diff, and would run svn
patch.
> When I make the change to $Id$ in sa-update-raw and run svn diff on my devel
> machine, svn outputs NOTHING as if the file hasn't even changed.
>
> On Minotaur, I see this SAME behavior as well so I can't even commit the change
> because SVN doesn't think a change occurred.
>
> my $VERSION = 'svn' . (split(/\s+/,
> '$Id: sa-update.raw 1028810 2010-10-29 15:43:10Z mmartinec $'))[2];
>
> So I'm guessing that is expected behavior because svn is sort of hiding this
> auto variable handling i.e. svn keywords.
Yup.
> Fourth, when I change to $Id$ and $LastChangeDate$, etc., I see the
> uninitialized errors you are talking about with make test.
Yup.
> Fifth, the patch doesn't compile due to a missing closing parens in this logic.
> I also changed some small things for consistency and attached a new version.
>
> + push(@EXTRA_VERSION,
> + ('r' . 'unknown');
> + }
Sorry, thanks for catching it.
> OK, so with that said, the conclusion is that I have made a patch and the
> various unitiliazed errors are gone. The trunk passes make test.
Awesome.
> By my theory and I think your explanation of svn keywords, once I commit this
> and run svn update, it should auto expand again... and then if I make, where
> versions look like this now:
>
> sa-update version svnunknown
>
> SpamAssassin version 3.4.0-rsvnunknown
>
> They'll change to something like svn<revision>.
Yup.
--
Configure bugmail: https://issues.apache.org/SpamAssassin/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
[Bug 6605] [review] Suboptimal results when svn keywords aren't
expanded
Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6605
--- Comment #14 from Kevin A. McGrail <km...@pccc.com> 2011-05-28 08:12:52 UTC ---
(In reply to comment #13)
> (In reply to comment #12)
> > Please let me know if the daily builds work now for you hence changing to
> > [review] statuso.
>
> Exactly as expected (version 3.4.0-0~15476~lucid1). I really didn't expect
> anyone else to do this much work on this, thanks.
Excellent. Thanks for the leg work on the bug. I never would have even
noticed the issue.
--
Configure bugmail: https://issues.apache.org/SpamAssassin/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
[Bug 6605] Suboptimal results when svn keywords aren't expanded
Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6605
Kevin A. McGrail <km...@pccc.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |kmcgrail@pccc.com
--- Comment #2 from Kevin A. McGrail <km...@pccc.com> 2011-05-27 21:12:07 UTC ---
Without testing of this in the Ubuntu daily build AND outside of that system,
this seems dangerously close to changes sole for a downstream package
requirement.
I give you extra credit because you at least submitted a patch (and said you
were going to).
I need some help with a test protocol if you want me to consider voting for
this patch:
a - Can you even test this without it going into trunk for launchpad?
b - How can I test that it doesn't break things outside of launchpad?
Regards,
KAM
--
Configure bugmail: https://issues.apache.org/SpamAssassin/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
[Bug 6605] Suboptimal results when svn keywords aren't expanded
Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6605
--- Comment #5 from Kevin A. McGrail <km...@pccc.com> 2011-05-27 21:48:12 UTC ---
(In reply to comment #4)
> And I don't think this needs voting, just a commit (after testing), since I
> don't have the access to do it myself.
True but it needs someone with commit status who agrees with the patch.
Right now, I can't even apply your patch and I'm unsure what your patch was
generated against.
My svn checkout sa-update.raw is:
my $VERSION = 'svn' . (split(/\s+/,
'$Id: sa-update.raw 1028810 2010-10-29 15:43:10Z mmartinec $'))[2];
Your patch, for example, refers to:
- my $VERSION = 'svn' . (split(/\s+/,
- '$Id$'))[2];
What am I missing?
--
Configure bugmail: https://issues.apache.org/SpamAssassin/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
[Bug 6605] Suboptimal results when svn keywords aren't expanded
Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6605
--- Comment #10 from Darxus <Da...@ChaosReigns.com> 2011-05-27 23:22:44 UTC ---
(In reply to comment #9)
> Are we on the same page and we are ready to try a commit as described?
Yes.
--
Configure bugmail: https://issues.apache.org/SpamAssassin/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
[Bug 6605] [review] Suboptimal results when svn keywords aren't
expanded
Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6605
Darxus <Da...@ChaosReigns.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |RESOLVED
Resolution| |FIXED
--- Comment #13 from Darxus <Da...@ChaosReigns.com> 2011-05-28 01:44:06 UTC ---
(In reply to comment #12)
> Please let me know if the daily builds work now for you hence changing to
> [review] statuso.
Exactly as expected (version 3.4.0-0~15476~lucid1). I really didn't expect
anyone else to do this much work on this, thanks.
Before:
# sa-update
Use of uninitialized value in concatenation (.) or string at /usr/bin/sa-update
line 23.
$ spamassassin < 1305216975.18582_3.panic:2,S | grep X-Spam-Checker-Version
X-Spam-Checker-Version: SpamAssassin 3.4.0-rupdated (updated) on
After:
# sa-update
$ spamassassin < 1305216975.18582_3.panic:2,S | grep X-Spam-Checker-Version
X-Spam-Checker-Version: SpamAssassin 3.4.0-rsvnunknown (svnunknown) on
--
Configure bugmail: https://issues.apache.org/SpamAssassin/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
[Bug 6605] Suboptimal results when svn keywords aren't expanded
Posted by bu...@bugzilla.spamassassin.org.
https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6605
Darxus <Da...@ChaosReigns.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |Darxus@ChaosReigns.com
--- Comment #3 from Darxus <Da...@ChaosReigns.com> 2011-05-27 21:28:58 UTC ---
(In reply to comment #2)
> Without testing of this in the Ubuntu daily build AND outside of that system,
> this seems dangerously close to changes sole for a downstream package
> requirement.
Yup. But it doesn't harm anything, it's well commented, and it increases the
chances I'll continue to test daily builds on my mail server.
> I need some help with a test protocol if you want me to consider voting for
> this patch:
>
> a - Can you even test this without it going into trunk for launchpad?
I could. I would need to create another svn repository, commit the change to
it, set up importation of that new repo on launchpad, and create a new build
recipe to build from it.
Fortunately, the only thing it can break that can't be tested without going
through all that is building Debian packages.
Which I'm comfortable risking for the amount of time it takes to commit to
trunk, tell launchpad to pull from trunk now, tell launchpad to rebuild the
packages now, install, and test them.
> b - How can I test that it doesn't break things outside of launchpad?
Install in the normal manner without launchpad. I'm planning to do this in
virtualbox later because installing from source makes me feel icky.
If a little clarification on the patch helps, its function is pretty simple:
There are three svn keywords involved. For each one, it checks to see if the
keyword has been expanded by checking for it to contain a ":". If it's there,
the behavior doesn't change. If it's not, it uses "unknown" instead of what it
would use if expansion had happened.
--
Configure bugmail: https://issues.apache.org/SpamAssassin/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.