Arrow_left   Arrow_right
 
  #90

clojure.contrib.io does not support appending anymore

    • Status: Test
    • Priority: Normal (3)
    • Component: -
    • Permission type: Private
    Reproduce the bug with:
    (require 'clojure.contrib.io)
(clojure.contrib.io/append-writer "/xyz.txt")

    which will throw an Exception “Cannot change an open stream to append mode”.

    This is caused because of a flaw in the call chain.
    clojure.contrib.io/writer for Strings calls again writer for whatever clojure.contrib.io/output-stream
    returns. We call append-writer which then calls writer on the String which in turn then calls:
    output-stream for ^String which calls
    output-stream for ^URL which calls
        output-stream for ^File which binds *append* to false and calls
            output-stream for ^OutputStream which
            returns a BufferedOutputStream instance which then jumps back to the
        ^File handler, which removes its binding for *append*!! and returns the BOS to the
    ^URL handler which returns the BOS to the
^String handler

    At this point the outer binding (for *append*) is set to true again, and assert-not-appending fails.
  • Followers
     
    Ico-users bpsm (Assigned To) , André Thieme 
     
    Attachments
    Fico_general
    0002-t90-default-reader-implementation-special-cases-for-.patch
    		1.45 KB Added by bpsm on August 26, 2010 UTC   Details
    Fico_general
    0001-t90-regression-test.patch
    		1010 Bytes Added by bpsm on August 26, 2010 UTC   Details
    Associations
     
    No associations
    Activity
     
    User picture

          on Aug 25, 2010 @ 03:56PM UTC * By bpsm

    Assigned to set to bpsm
    Status changed from New to Accepted
    I've looked at this defect (on master). It seems thorny to me.

    I'm finding the control flow in c.c.io rather difficult to follow, what with all those extensions of the Streams protocol delegating to one-another. I'm reasonably certain that the solution is to special case the creation of a reader in append mode.

    I'll attach a patch that does this by providing a default implementation for :writer, which seems to solve the problem, but I think I still need to think about it some more before I can call it 'done'.

    Note: this defect is also present in clojure.java.io 1.2.0.
    User picture

          on Aug 25, 2010 @ 04:12PM UTC * By bpsm

    Attachment 0001-t90-default-reader-implementation-special-cases-for-.patch added
    file:auuXEWSiur35DyeJe5cbCb
    User picture

          on Aug 25, 2010 @ 04:26PM UTC * By bpsm

    Correction: This issue is not present in clojure 1.2.0. my bad.
    I was mistaken about this bug being present in clojure.java.io. append-writer doesn't even exist there and the append magic is gone there too as per clojure#311.
    I think there was just something wonky with my swank-clojure:
    clojure.contrib.io> (require 'clojure.java.io)
nil
clojure.contrib.io> (in-ns 'clojure.java.io)
#<Namespace clojure.java.io>
clojure.java.io> (with-open [x (append-writer "/tmp/foo.txt")] x)
; Evaluation aborted.  ;;; EXCEPTION HERE
    User picture

          on Aug 26, 2010 @ 02:23PM UTC * By bpsm

    Attachment 0001-t90-regression-test.patch added
    file:amTyVwSt8r378NeJe5cbLr: regression test
    User picture

          on Aug 26, 2010 @ 02:25PM UTC * By bpsm

    Status changed from Accepted to Test
    The provided regression test demonstrates the described issue and confirms that the provided patch resolves the issue. All other io unit tests continue to pass.
    User picture

          on Aug 28, 2010 @ 01:02AM UTC * By stuartsierra

    clojure.contrib.io is deprecated and will be removed before the next release.
    User picture

          on Aug 28, 2010 @ 03:03AM UTC * By bpsm

    Let's see if I'm on the same page that you're on:

    Since 1.2.0 is at RC3 already, you must mean 1.3.0 when you say "next release", yes/no? Predicated on that:

    • I think this should be fixed on the 1.2.x branch, where c.c.io is available, though deprecated. ("deprecated, but working" is better than "deprecated by slightly buggy.") The attached patch is against 1.2.x, not master, despite what my initial comment on this issue says.
    • I've provided a patch on #93 to remove c.c.io on master (1.3.x).
    Time Expenditure
    Loading