From: Pierre R. M. <pm...@pm...> - 2008-07-24 15:05:36
|
Since sb-cover:report expects the given directory argument to be a pathname that it can bind to *default-pathname-defaults*, it has to ensure that it actually is a directory pathname, otherwise e.g. ensure-directories- exist will break with confusing errors due to breakage in probe-file (probe- file will merge its argument against *default-pathname-defaults* again, which will turn e.g. #P"/" -- the first dir probed by ensure-directories- exist -- into a different pathname if the *d-p-d* contains a filename component). --- contrib/sb-cover/cover.lisp | 14 +++++++++++++- 1 files changed, 13 insertions(+), 1 deletions(-) diff --git a/contrib/sb-cover/cover.lisp b/contrib/sb-cover/cover.lisp index c273b86..8c35986 100644 --- a/contrib/sb-cover/cover.lisp +++ b/contrib/sb-cover/cover.lisp @@ -79,6 +79,17 @@ result to RESTORE-COVERAGE." (restore-coverage (read stream)))) (values))) +(defun pathname-as-directory (pathname) + (let ((pathname (pathname pathname))) + (if (and (member (pathname-name pathname) '(nil :unspecific)) + (member (pathname-name pathname) '(nil :unspecific))) + pathname + (make-pathname :directory (append (or (pathname-directory pathname) + (list :relative)) + (list (file-namestring pathname))) + :name nil :type nil + :defaults pathname)))) + (defun report (directory &key ((:form-mode *source-path-mode*) :whole) (external-format :default)) "Print a code coverage report of all instrumented files into DIRECTORY. @@ -92,7 +103,8 @@ it has the value :WHOLE the whole form will be annotated (the default). The former mode shows explicitly which forms were instrumented, while the latter mode is generally easier to read." (let ((paths) - (*default-pathname-defaults* (merge-pathnames (pathname directory)))) + (*default-pathname-defaults* + (pathname-as-directory (merge-pathnames (pathname directory))))) (ensure-directories-exist *default-pathname-defaults*) (maphash (lambda (k v) (declare (ignore v)) -- Pierre R. Mai <pm...@pm...> PMSF IT Consulting Pierre R. Mai http://www.pmsf.de/ Ludwig-Thoma-Str. 11 Schleißheimer Str. 263 87724 Ottobeuren 80809 München Tel. +49(0)8332/93669-13 +49(0)89/35061750 Fax +49(0)8332/93669-03 Germany |
From: Pierre R. M. <pm...@pm...> - 2008-07-24 21:44:51
|
Am 24.07.2008 um 16:43 schrieb Pierre R. Mai: > > + (if (and (member (pathname-name pathname) '(nil :unspecific)) > + (member (pathname-name pathname) '(nil :unspecific))) As Andreas Franke rightly has pointed out this patch contains a copy&paste bug (and some tab breakage), please use the ammended patch in the following mail instead, sorry for the noise. Regs, Pierre. -- Pierre R. Mai <pm...@pm...> PMSF IT Consulting Pierre R. Mai http://www.pmsf.de/ Ludwig-Thoma-Str. 11 Schleißheimer Str. 263 87724 Ottobeuren 80809 München Tel. +49(0)8332/93669-13 +49(0)89/35061750 Fax +49(0)8332/93669-03 Germany |
From: Pierre R. M. <pm...@pm...> - 2008-07-24 21:49:57
Attachments:
50ac8be47967c01ef57e6c872c9b952c8ff32516.diff
|
Since sb-cover:report expects the given directory argument to be a pathname that it can bind to *default-pathname-defaults*, it has to ensure that it actually is a directory pathname, otherwise e.g. ensure-directories-exist will break with confusing errors due to breakage in probe-file (probe-file will merge its argument against *default-pathname-defaults* again, which will turn #P"/" -- the first dir probed by ensure-directories-exist -- into a different pathname if the *d-p-d* contains a filename component). --- contrib/sb-cover/cover.lisp | 14 +++++++++++++- 1 files changed, 13 insertions(+), 1 deletions(-) |
From: Nikodemus S. <nik...@ra...> - 2008-07-30 14:49:01
|
On Fri, Jul 25, 2008 at 12:49 AM, Pierre R. Mai <pm...@pm...> wrote: > > Since sb-cover:report expects the given directory argument to be a pathname > that it can bind to *default-pathname-defaults*, it has to ensure that it > actually is a directory pathname, otherwise e.g. ensure-directories-exist > will break with confusing errors due to breakage in probe-file (probe-file > will merge its argument against *default-pathname-defaults* again, which > will turn #P"/" -- the first dir probed by ensure-directories-exist -- into > a different pathname if the *d-p-d* contains a filename component). Thank you for the patch. I was wondering if a more conservative approach of signaling an error for non-directory pathname in REPORT would be better? Otherwise "/home/me/coverage" will cause REPORT barf all over the poor home directory. Cheers, -- Nikodemus |
From: Pierre R. M. <pm...@pm...> - 2008-07-30 15:24:43
|
Am 30.07.2008 um 16:48 schrieb Nikodemus Siivola: > On Fri, Jul 25, 2008 at 12:49 AM, Pierre R. Mai <pm...@pm...> wrote: >> >> Since sb-cover:report expects the given directory argument to be a >> pathname >> that it can bind to *default-pathname-defaults*, it has to ensure >> that it >> actually is a directory pathname, otherwise e.g. ensure-directories- >> exist >> will break with confusing errors due to breakage in probe-file >> (probe-file >> will merge its argument against *default-pathname-defaults* again, >> which >> will turn #P"/" -- the first dir probed by ensure-directories-exist >> -- into >> a different pathname if the *d-p-d* contains a filename component). > > Thank you for the patch. I was wondering if a more conservative > approach of signaling an error for non-directory pathname in REPORT > would be better? Otherwise > > "/home/me/coverage" > > will cause REPORT barf all over the poor home directory. Actually, the patch as implemented would not barf over the home directory (assuming we are talking about user me, not user coverage ;), but would treat this as if the user had supplied "/home/me/coverage/" i.e. ensure directory coverage exists, and then put the report in there. I agree that this is a bit of DWIM-like behavior, but then again sb- cover:report is probably intended for interactive use... Personally I think that interfaces that require a directory pathname are too fragile for my taste, so maybe it would be better to specify that report should be given the pathname for the index.html file, and put the secondary files generated into the same directory? Regs, Pierre. -- Pierre R. Mai <pm...@pm...> PMSF IT Consulting Pierre R. Mai http://www.pmsf.de/ Ludwig-Thoma-Str. 11 Schleißheimer Str. 263 87724 Ottobeuren 80809 München Tel. +49(0)8332/93669-13 +49(0)89/35061750 Fax +49(0)8332/93669-03 Germany |
From: Nikodemus S. <nik...@ra...> - 2008-07-31 06:53:58
|
On Wed, Jul 30, 2008 at 6:24 PM, Pierre R. Mai <pm...@pm...> wrote: > Actually, the patch as implemented would not barf over the home directory > (assuming we are talking about user me, not user coverage ;), but would > treat this as if the user had supplied > > "/home/me/coverage/" > > i.e. ensure directory coverage exists, and then put the report in there. > > I agree that this is a bit of DWIM-like behavior, but then again > sb-cover:report is probably intended for interactive use... Oops, read it too quick obviously. I'm not sure about the interactive use, though: one of the more interesting ones uses for SB-COVER is running as part of the normal build + test cycle, so that you have a record of how your coverage grows or shrinks. I merged a version which signals an error as 1.0.19.8 -- but modding it to provide the DWIM behaviour is just a matter passing NIL as ERRORP. (Not part of the REPORT API.) > Personally I think that interfaces that require a directory pathname are too > fragile for my taste, so maybe it would be better to specify that report > should be given the pathname for the index.html file, and put the secondary > files generated into the same directory? Dunno. Cheers, -- Nikodemus |