From dc966032129e07b9a6002a97bc9e6b86c712b4d2 Mon Sep 17 00:00:00 2001 From: "http://kerravonsen.dreamwidth.org/" Date: Thu, 25 Mar 2010 03:35:14 +0000 Subject: bug and fix --- doc/bugs/filecheck_failing_to_find_files.mdwn | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 doc/bugs/filecheck_failing_to_find_files.mdwn (limited to 'doc/bugs/filecheck_failing_to_find_files.mdwn') diff --git a/doc/bugs/filecheck_failing_to_find_files.mdwn b/doc/bugs/filecheck_failing_to_find_files.mdwn new file mode 100644 index 000000000..95ea5c763 --- /dev/null +++ b/doc/bugs/filecheck_failing_to_find_files.mdwn @@ -0,0 +1,23 @@ +Using the attachment plugin, when filecheck was checking the mime-type of the attachment before allowing the attachment to be removed, it was returning with an error saying that the mime-type of the file was "unknown" (when the mime-type definitely was known!) + +It turns out that the filecheck plugin couldn't find the file, because it was merely using the $pagesources hash, rather than finding the absolute path of the file in question. + +The following patch fixes the problem: + + diff --git a/IkiWiki/Plugin/filecheck.pm b/IkiWiki/Plugin/filecheck.pm + index 01d4909..1cec0cf 100644 + --- a/IkiWiki/Plugin/filecheck.pm + +++ b/IkiWiki/Plugin/filecheck.pm + @@ -118,6 +118,10 @@ sub match_mimetype ($$;@) { + if (! defined $file) { + return IkiWiki::ErrorReason->new("no file specified"); + } + + if (! -e $file) { + + # get the absolute path of the file if you can't find it + + $file = IkiWiki::srcfile($file); + + } + + # Use ::magic to get the mime type, the idea is to only trust + # data obtained by examining the actual file contents. + +[[!tag patch]] -- cgit v1.2.3 From c757cfa64f56fc17b2c3e4b2c36279f3fcf0f812 Mon Sep 17 00:00:00 2001 From: "http://kerravonsen.dreamwidth.org/" Date: Thu, 25 Mar 2010 03:38:14 +0000 Subject: I'm not sure if I'm supposed to tag this as patch, so I removed the tag. --- doc/bugs/filecheck_failing_to_find_files.mdwn | 2 -- 1 file changed, 2 deletions(-) (limited to 'doc/bugs/filecheck_failing_to_find_files.mdwn') diff --git a/doc/bugs/filecheck_failing_to_find_files.mdwn b/doc/bugs/filecheck_failing_to_find_files.mdwn index 95ea5c763..49de3d435 100644 --- a/doc/bugs/filecheck_failing_to_find_files.mdwn +++ b/doc/bugs/filecheck_failing_to_find_files.mdwn @@ -19,5 +19,3 @@ The following patch fixes the problem: # Use ::magic to get the mime type, the idea is to only trust # data obtained by examining the actual file contents. - -[[!tag patch]] -- cgit v1.2.3 From 1030d23a2ca1be2680a6fac8366f880a1d1760c6 Mon Sep 17 00:00:00 2001 From: Joey Hess Date: Thu, 25 Mar 2010 00:11:39 -0400 Subject: response --- doc/bugs/filecheck_failing_to_find_files.mdwn | 3 +++ 1 file changed, 3 insertions(+) (limited to 'doc/bugs/filecheck_failing_to_find_files.mdwn') diff --git a/doc/bugs/filecheck_failing_to_find_files.mdwn b/doc/bugs/filecheck_failing_to_find_files.mdwn index 49de3d435..e41793ee8 100644 --- a/doc/bugs/filecheck_failing_to_find_files.mdwn +++ b/doc/bugs/filecheck_failing_to_find_files.mdwn @@ -2,6 +2,9 @@ Using the attachment plugin, when filecheck was checking the mime-type of the at It turns out that the filecheck plugin couldn't find the file, because it was merely using the $pagesources hash, rather than finding the absolute path of the file in question. +> I don't understand why the file was not in `%pagesources`. Do you? +> --[[Joey]] + The following patch fixes the problem: diff --git a/IkiWiki/Plugin/filecheck.pm b/IkiWiki/Plugin/filecheck.pm -- cgit v1.2.3 From 46f8f72793e23b56d3deb1ed0ed6b1a0fbd56ad6 Mon Sep 17 00:00:00 2001 From: "http://kerravonsen.dreamwidth.org/" Date: Thu, 25 Mar 2010 04:43:16 +0000 Subject: response --- doc/bugs/filecheck_failing_to_find_files.mdwn | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) (limited to 'doc/bugs/filecheck_failing_to_find_files.mdwn') diff --git a/doc/bugs/filecheck_failing_to_find_files.mdwn b/doc/bugs/filecheck_failing_to_find_files.mdwn index e41793ee8..be6cdbb20 100644 --- a/doc/bugs/filecheck_failing_to_find_files.mdwn +++ b/doc/bugs/filecheck_failing_to_find_files.mdwn @@ -3,7 +3,12 @@ Using the attachment plugin, when filecheck was checking the mime-type of the at It turns out that the filecheck plugin couldn't find the file, because it was merely using the $pagesources hash, rather than finding the absolute path of the file in question. > I don't understand why the file was not in `%pagesources`. Do you? -> --[[Joey]] +> --[[Joey]] + +>> The file *was* in `%pagesources`, but what returns from that is the filename relative to the `srcdir` directory; for example, `foo/bar.gif`. +>> When File::MimeInfo::Magic::magic is given that, it can't find the file. +>> But if it is given `/path/to/srcdir/foo/bar.gif` instead, then it *can* find the file, and returns the mime-type correctly. +>> --[[KathrynAndersen]] The following patch fixes the problem: -- cgit v1.2.3 From 3d671ea8c1df4534d8ffa59b235dd6ded99bb13f Mon Sep 17 00:00:00 2001 From: Joey Hess Date: Thu, 25 Mar 2010 14:39:09 -0400 Subject: filecheck: Fix bug that prevented the pagespecs from matching when not called by attachment plugin. --- IkiWiki/Plugin/filecheck.pm | 16 ++++++++-------- debian/changelog | 2 ++ doc/bugs/filecheck_failing_to_find_files.mdwn | 5 +++++ 3 files changed, 15 insertions(+), 8 deletions(-) (limited to 'doc/bugs/filecheck_failing_to_find_files.mdwn') diff --git a/IkiWiki/Plugin/filecheck.pm b/IkiWiki/Plugin/filecheck.pm index 0501ba99c..1549b82db 100644 --- a/IkiWiki/Plugin/filecheck.pm +++ b/IkiWiki/Plugin/filecheck.pm @@ -75,9 +75,9 @@ sub match_maxsize ($$;@) { } my %params=@_; - my $file=exists $params{file} ? $params{file} : $IkiWiki::pagesources{$page}; + my $file=exists $params{file} ? $params{file} : IkiWiki::srcfile($IkiWiki::pagesources{$page}); if (! defined $file) { - return IkiWiki::ErrorReason->new("no file specified"); + return IkiWiki::ErrorReason->new("file does not exist"); } if (-s $file > $maxsize) { @@ -96,9 +96,9 @@ sub match_minsize ($$;@) { } my %params=@_; - my $file=exists $params{file} ? $params{file} : $IkiWiki::pagesources{$page}; + my $file=exists $params{file} ? $params{file} : IkiWiki::srcfile($IkiWiki::pagesources{$page}); if (! defined $file) { - return IkiWiki::ErrorReason->new("no file specified"); + return IkiWiki::ErrorReason->new("file does not exist"); } if (-s $file < $minsize) { @@ -114,9 +114,9 @@ sub match_mimetype ($$;@) { my $wanted=shift; my %params=@_; - my $file=exists $params{file} ? $params{file} : $IkiWiki::pagesources{$page}; + my $file=exists $params{file} ? $params{file} : IkiWiki::srcfile($IkiWiki::pagesources{$page}); if (! defined $file) { - return IkiWiki::ErrorReason->new("no file specified"); + return IkiWiki::ErrorReason->new("file does not exist"); } # Use ::magic to get the mime type, the idea is to only trust @@ -147,9 +147,9 @@ sub match_virusfree ($$;@) { my $wanted=shift; my %params=@_; - my $file=exists $params{file} ? $params{file} : $IkiWiki::pagesources{$page}; + my $file=exists $params{file} ? $params{file} : IkiWiki::srcfile($IkiWiki::pagesources{$page}); if (! defined $file) { - return IkiWiki::ErrorReason->new("no file specified"); + return IkiWiki::ErrorReason->new("file does not exist"); } if (! exists $IkiWiki::config{virus_checker} || diff --git a/debian/changelog b/debian/changelog index 12dd0dc02..7249cdfa4 100644 --- a/debian/changelog +++ b/debian/changelog @@ -15,6 +15,8 @@ ikiwiki (3.20100324) UNRELEASED; urgency=low * Allow wrappers to be built using tcc. * Add support for setup files written in YAML. * Add --set-yaml switch for setting more complex config file options. + * filecheck: Fix bug that prevented the pagespecs from matching when + not called by attachment plugin. -- Joey Hess Sat, 13 Mar 2010 14:48:10 -0500 diff --git a/doc/bugs/filecheck_failing_to_find_files.mdwn b/doc/bugs/filecheck_failing_to_find_files.mdwn index be6cdbb20..e896f2129 100644 --- a/doc/bugs/filecheck_failing_to_find_files.mdwn +++ b/doc/bugs/filecheck_failing_to_find_files.mdwn @@ -10,6 +10,11 @@ It turns out that the filecheck plugin couldn't find the file, because it was me >> But if it is given `/path/to/srcdir/foo/bar.gif` instead, then it *can* find the file, and returns the mime-type correctly. >> --[[KathrynAndersen]] +>>> Ok, so it's not removal specific, can in fact be triggered by using +>>> testpagespec (or really anything besides attachment, which passes +>>> the filename parameter). Nor is it limited to mimetype, all the tests in +>>> filecheck have the problem. [[Fixed|done]] --[[Joey]] + The following patch fixes the problem: diff --git a/IkiWiki/Plugin/filecheck.pm b/IkiWiki/Plugin/filecheck.pm -- cgit v1.2.3 From b68aa11732a1782886793e426f06652656c155cb Mon Sep 17 00:00:00 2001 From: "http://kerravonsen.dreamwidth.org/" Date: Fri, 26 Mar 2010 01:40:32 +0000 Subject: oh dear not fixed after all --- doc/bugs/filecheck_failing_to_find_files.mdwn | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) (limited to 'doc/bugs/filecheck_failing_to_find_files.mdwn') diff --git a/doc/bugs/filecheck_failing_to_find_files.mdwn b/doc/bugs/filecheck_failing_to_find_files.mdwn index e896f2129..33975f05c 100644 --- a/doc/bugs/filecheck_failing_to_find_files.mdwn +++ b/doc/bugs/filecheck_failing_to_find_files.mdwn @@ -13,7 +13,13 @@ It turns out that the filecheck plugin couldn't find the file, because it was me >>> Ok, so it's not removal specific, can in fact be triggered by using >>> testpagespec (or really anything besides attachment, which passes >>> the filename parameter). Nor is it limited to mimetype, all the tests in ->>> filecheck have the problem. [[Fixed|done]] --[[Joey]] +>>> filecheck have the problem. --[[Joey]] + +>>>> Alas, not fixed. It seems I was mistaken in some of my assumptions. +>>>> It still happens when attempting to remove attachments. +>>>> With your fix, the `IkiWiki::srcfile` function is only called when the filename is not passed in, but it appears that in the case of removing attachments, the filename IS passed in, but it is the relative filename as mentioned above. Thus, the file is still not found, and the mime-type comes back as unknown. +>>>> The reason my patch worked is because, rather than checking whether a filename was passed in before applying IkiWiki::srcfile to the filename, it checks whether the file can be found, and if it cannot be found, then it applies IkiWiki::srcfile to the filename. +>>>> --[[KathrynAndersen]] The following patch fixes the problem: -- cgit v1.2.3 From fb39dc5f5f3e0dc4b5006b2cf7c85ce0e895ebfa Mon Sep 17 00:00:00 2001 From: Joey Hess Date: Thu, 25 Mar 2010 23:40:38 -0400 Subject: patch --- doc/bugs/filecheck_failing_to_find_files.mdwn | 53 +++++++++++++++++++-------- 1 file changed, 37 insertions(+), 16 deletions(-) (limited to 'doc/bugs/filecheck_failing_to_find_files.mdwn') diff --git a/doc/bugs/filecheck_failing_to_find_files.mdwn b/doc/bugs/filecheck_failing_to_find_files.mdwn index 33975f05c..3209c8f53 100644 --- a/doc/bugs/filecheck_failing_to_find_files.mdwn +++ b/doc/bugs/filecheck_failing_to_find_files.mdwn @@ -21,20 +21,41 @@ It turns out that the filecheck plugin couldn't find the file, because it was me >>>> The reason my patch worked is because, rather than checking whether a filename was passed in before applying IkiWiki::srcfile to the filename, it checks whether the file can be found, and if it cannot be found, then it applies IkiWiki::srcfile to the filename. >>>> --[[KathrynAndersen]] -The following patch fixes the problem: +>>>>> Can you test if this patch fixes that? --[[Joey]] - diff --git a/IkiWiki/Plugin/filecheck.pm b/IkiWiki/Plugin/filecheck.pm - index 01d4909..1cec0cf 100644 - --- a/IkiWiki/Plugin/filecheck.pm - +++ b/IkiWiki/Plugin/filecheck.pm - @@ -118,6 +118,10 @@ sub match_mimetype ($$;@) { - if (! defined $file) { - return IkiWiki::ErrorReason->new("no file specified"); - } - + if (! -e $file) { - + # get the absolute path of the file if you can't find it - + $file = IkiWiki::srcfile($file); - + } - - # Use ::magic to get the mime type, the idea is to only trust - # data obtained by examining the actual file contents. +
+diff --git a/IkiWiki/Plugin/remove.pm b/IkiWiki/Plugin/remove.pm
+index f59d026..0fc180f 100644
+--- a/IkiWiki/Plugin/remove.pm
++++ b/IkiWiki/Plugin/remove.pm
+@@ -49,7 +49,7 @@ sub check_canremove ($$$) {
+ 	# This is sorta overkill, but better safe than sorry.
+ 	if (! defined pagetype($pagesources{$page})) {
+ 		if (IkiWiki::Plugin::attachment->can("check_canattach")) {
+-			IkiWiki::Plugin::attachment::check_canattach($session, $page, $file);
++			IkiWiki::Plugin::attachment::check_canattach($session, $page, "$config{srcdir}/$file");
+ 		}
+ 		else {
+ 			error("removal of attachments is not allowed");
+diff --git a/IkiWiki/Plugin/rename.pm b/IkiWiki/Plugin/rename.pm
+index 3908443..1a9da63 100644
+--- a/IkiWiki/Plugin/rename.pm
++++ b/IkiWiki/Plugin/rename.pm
+@@ -50,7 +50,7 @@ sub check_canrename ($$$$$$) {
+ 	IkiWiki::check_canedit($src, $q, $session);
+ 	if ($attachment) {
+ 		if (IkiWiki::Plugin::attachment->can("check_canattach")) {
+-			IkiWiki::Plugin::attachment::check_canattach($session, $src, $srcfile);
++			IkiWiki::Plugin::attachment::check_canattach($session, $src, "$config{srcdir}/$srcfile");
+ 		}
+ 		else {
+ 			error("renaming of attachments is not allowed");
+@@ -85,7 +85,7 @@ sub check_canrename ($$$$$$) {
+ 		if ($attachment) {
+ 			# Note that $srcfile is used here, not $destfile,
+ 			# because it wants the current file, to check it.
+-			IkiWiki::Plugin::attachment::check_canattach($session, $dest, $srcfile);
++			IkiWiki::Plugin::attachment::check_canattach($session, $dest, "$config{srcdir}/$srcfile");
+ 		}
+ 	}
+
-- cgit v1.2.3 From 62306d85f998b5bbd64d213e8c591809dbc07cb8 Mon Sep 17 00:00:00 2001 From: "http://kerravonsen.dreamwidth.org/" Date: Fri, 26 Mar 2010 04:06:31 +0000 Subject: It works! --- doc/bugs/filecheck_failing_to_find_files.mdwn | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'doc/bugs/filecheck_failing_to_find_files.mdwn') diff --git a/doc/bugs/filecheck_failing_to_find_files.mdwn b/doc/bugs/filecheck_failing_to_find_files.mdwn index 3209c8f53..f8d8e83e6 100644 --- a/doc/bugs/filecheck_failing_to_find_files.mdwn +++ b/doc/bugs/filecheck_failing_to_find_files.mdwn @@ -21,7 +21,9 @@ It turns out that the filecheck plugin couldn't find the file, because it was me >>>> The reason my patch worked is because, rather than checking whether a filename was passed in before applying IkiWiki::srcfile to the filename, it checks whether the file can be found, and if it cannot be found, then it applies IkiWiki::srcfile to the filename. >>>> --[[KathrynAndersen]] ->>>>> Can you test if this patch fixes that? --[[Joey]] +>>>>> Can you test if this patch fixes that? --[[Joey]] + +>>>>>> Yes, it works! --[[KathrynAndersen]]
 diff --git a/IkiWiki/Plugin/remove.pm b/IkiWiki/Plugin/remove.pm
-- 
cgit v1.2.3


From 243b0dd082cf4b66dfef55b2c9b459109bee7398 Mon Sep 17 00:00:00 2001
From: Joey Hess 
Date: Fri, 26 Mar 2010 00:16:21 -0400
Subject: fix the other half of the filecheck filename bug

---
 IkiWiki/Plugin/remove.pm                      | 2 +-
 IkiWiki/Plugin/rename.pm                      | 4 ++--
 debian/changelog                              | 2 +-
 doc/bugs/filecheck_failing_to_find_files.mdwn | 2 ++
 4 files changed, 6 insertions(+), 4 deletions(-)

(limited to 'doc/bugs/filecheck_failing_to_find_files.mdwn')

diff --git a/IkiWiki/Plugin/remove.pm b/IkiWiki/Plugin/remove.pm
index f59d0269e..0fc180f69 100644
--- a/IkiWiki/Plugin/remove.pm
+++ b/IkiWiki/Plugin/remove.pm
@@ -49,7 +49,7 @@ sub check_canremove ($$$) {
 	# This is sorta overkill, but better safe than sorry.
 	if (! defined pagetype($pagesources{$page})) {
 		if (IkiWiki::Plugin::attachment->can("check_canattach")) {
-			IkiWiki::Plugin::attachment::check_canattach($session, $page, $file);
+			IkiWiki::Plugin::attachment::check_canattach($session, $page, "$config{srcdir}/$file");
 		}
 		else {
 			error("removal of attachments is not allowed");
diff --git a/IkiWiki/Plugin/rename.pm b/IkiWiki/Plugin/rename.pm
index 3908443ca..1a9da6363 100644
--- a/IkiWiki/Plugin/rename.pm
+++ b/IkiWiki/Plugin/rename.pm
@@ -50,7 +50,7 @@ sub check_canrename ($$$$$$) {
 	IkiWiki::check_canedit($src, $q, $session);
 	if ($attachment) {
 		if (IkiWiki::Plugin::attachment->can("check_canattach")) {
-			IkiWiki::Plugin::attachment::check_canattach($session, $src, $srcfile);
+			IkiWiki::Plugin::attachment::check_canattach($session, $src, "$config{srcdir}/$srcfile");
 		}
 		else {
 			error("renaming of attachments is not allowed");
@@ -85,7 +85,7 @@ sub check_canrename ($$$$$$) {
 		if ($attachment) {
 			# Note that $srcfile is used here, not $destfile,
 			# because it wants the current file, to check it.
-			IkiWiki::Plugin::attachment::check_canattach($session, $dest, $srcfile);
+			IkiWiki::Plugin::attachment::check_canattach($session, $dest, "$config{srcdir}/$srcfile");
 		}
 	}
 
diff --git a/debian/changelog b/debian/changelog
index 7249cdfa4..da1ab890e 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -15,7 +15,7 @@ ikiwiki (3.20100324) UNRELEASED; urgency=low
   * Allow wrappers to be built using tcc.
   * Add support for setup files written in YAML.
   * Add --set-yaml switch for setting more complex config file options.
-  * filecheck: Fix bug that prevented the pagespecs from matching when
+  * filecheck: Fix bugs that prevented the pagespecs from matching when
     not called by attachment plugin.
 
  -- Joey Hess   Sat, 13 Mar 2010 14:48:10 -0500
diff --git a/doc/bugs/filecheck_failing_to_find_files.mdwn b/doc/bugs/filecheck_failing_to_find_files.mdwn
index f8d8e83e6..6501508e4 100644
--- a/doc/bugs/filecheck_failing_to_find_files.mdwn
+++ b/doc/bugs/filecheck_failing_to_find_files.mdwn
@@ -25,6 +25,8 @@ It turns out that the filecheck plugin couldn't find the file, because it was me
 
 >>>>>> Yes, it works! --[[KathrynAndersen]]
 
+applied && [[done]]
+
 
 diff --git a/IkiWiki/Plugin/remove.pm b/IkiWiki/Plugin/remove.pm
 index f59d026..0fc180f 100644
-- 
cgit v1.2.3