1208898 - sdk/io/file does not allow opening in append mode

Status: RESOLVED INCOMPLETE

(Add-on SDK Graveyard :: General, defect)

Description

[erjoalgo]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Firefox/31.0 Iceweasel/31.8.0
Build ID: 20150701023259

Steps to reproduce:

util/io is a thin wrapper around sdk/io/file

exports["test file-append-test"] = function(assert, done) {
    var io = require("../util/io.js");
    var io = require("util/io.js");
    // var io = require("io");//doesn't work
    var stringa = "hola",
	stringb = "adios",
	fn = "/tmp/jpm-file-test";
    
    io.writeTextToFile(fn, stringa);
    io.writeTextToFile(fn, stringb, "a");
    var contents = io.readTextFromFile(fn);
    var expected = stringa+stringb;
    assert.equal(contents, expected);
    done();
}


Actual results:

no append


Expected results:

append

Comment 1

[Adrian Aichner [:anaran]]

You are redeclaring var io.

The first definition should be correct, assuming "util" is a sibling directory of the add-on's data directory.

Comment 2

[erjoalgo]

Sorry, that test is within my own addon, which defines its own io. I just included it to intuitively show what is wrong. 

I made a pull request here, tested it locally.

https://github.com/erjoalgo/addon-sdk/commit/dbe6d23bd727d06e54075a3d88a33ef8939f56b7

Comment 3

[erjoalgo]

Here's the fix that I'm using now which is the essence of the PR:

exports.open = function open(filename, mode) {
  var file = MozFile(filename);
  if (typeof(mode) !== "string")
    mode = "";

  // File opened for write only.
    if (/[aw]/.test(mode)) {
	var exists = file.exists();
    if (exists)
	ensureFile(file);
    var stream = Cc['@mozilla.org/network/file-output-stream;1'].
                 createInstance(Ci.nsIFileOutputStream);
      var openFlags = OPEN_FLAGS.WRONLY;
        
      if (exists && /a/.test(mode))	{
	  openFlags |= OPEN_FLAGS.APPEND;
      }else 	{
	  openFlags|= OPEN_FLAGS.TRUNCATE|
	      OPEN_FLAGS.CREATE_FILE;
      }

    var permFlags = parseInt("0644", 8); // u+rw go+r
    try {
      stream.init(file, openFlags, permFlags, 0);
    }
    catch (err) {
      throw friendlyError(err, filename);
    }
    return /b/.test(mode) ?
           new byteStreams.ByteWriter(stream) :
           new textStreams.TextWriter(stream);
  }

  // File opened for read only, the default.
  ensureFile(file);
  stream = Cc['@mozilla.org/network/file-input-stream;1'].
           createInstance(Ci.nsIFileInputStream);
  try {
    stream.init(file, OPEN_FLAGS.RDONLY, 0, 0);
  }
  catch (err) {
    throw friendlyError(err, filename);
  }
  return /b/.test(mode) ?
         new byteStreams.ByteReader(stream) :
         new textStreams.TextReader(stream);
};

Comment 4

[erjoalgo]

Attachment: Bug-1208898.patch patch for missing sdk/io/file.open append mode

Comment 5

[erjoalgo]

added tests and squashed to one commit, as per the contributor instructions
(previous commit did not include tests, please disregard)
https://github.com/mozilla/addon-sdk/pull/2034

Comment 6

[Adrian Aichner [:anaran]]

I assigned you the bug.
You can add a link to your pull request as an attachment to this bug and obsolete the previous attachments.
I can add commits to that same pull request should more changes be needed.

Comment 7

[Adrian Aichner [:anaran]]

Oops, meant to say "You can add commits ..."

Comment 8

[erjoalgo]

It should be the same pull request:
https://github.com/mozilla/addon-sdk/pull/2034/

I fixed the formatting issues. Let me know if you notice anything else.

Comment 9

[erjoalgo]

Can someone take a look at this? I added a pull request last weeek and only got  Adrian Aichner [:anaran]'s code formatting comments, which I've since fixed.

Comment 10

[Dave Townsend [:mossop]]

Attachment: pull request

Unfortunately I'd rather not take most of the style changes in this patch, I'd prefer to leave blame intact than follow the style guide to the letter. It looks like you should be able to implement this with just a couple of changes to choose between OPEN_FLAGS.TRUNCATE and OPEN_FLAGS.APPEND.

Re-request review on this when you've updated it, but don't worry about squashing the changes, I'll do that when it lands.

Comment 11

[Firefox Bug Husbandry Bot]

https://bugzilla.mozilla.org/show_bug.cgi?id=1399562