rake-pipeline icon indicating copy to clipboard operation
rake-pipeline copied to clipboard

Ensure files are read with the proper encoding

Open joefiorini opened this issue 12 years ago • 11 comments

This fixed the problems I had with copying binary files through the pipeline.

joefiorini avatar Oct 27 '12 14:10 joefiorini

How can I review this pull request?

I am not a Ruby specialist... can I pull the changes directly from the gem location and still get updates for new versions? What would be the git commands for this?

The gem location in my environment is: C:\RailsInstaller\Ruby1.9.3\lib\ruby\gems\1.9.1\bundler\gems\rake-pipeline-65b1e744defa

Tks

ronaldocpontes avatar Apr 29 '13 12:04 ronaldocpontes

I am still getting corrupted images...

  • copied your changed "file_wrapper.rb" into C:\RailsInstaller\Ruby1.9.3\lib\ruby\gems\1.9.1\bundler\gems\rake-pipeline-65b1e744defa\lib\rake-pipeline
  • deleted the assets and tmp folder
  • ran bundle exec rake build

This is what I am using in my Assetfile:

concatFilter = Rake::Pipeline::ConcatFilter concatFilter.processes_binary_files match "static/*/" do filter concatFilter do |input| input.sub(/static//, '') end end

Note: I am sure your changes are being picked up because I get an error if I purposely corrupt "file_wrapper.rb"

ronaldocpontes avatar Apr 29 '13 12:04 ronaldocpontes

Can you post an image that's getting corrupted (pre-corruption) here so I can try it on my machine?

Thanks!

On Apr 29, 2013, at 8:46 AM, ronaldocpontes [email protected] wrote:

I am still getting corrupted images...

copied your changed "file_wrapper.rb" into C:\RailsInstaller\Ruby1.9.3\lib\ruby\gems\1.9.1\bundler\gems\rake-pipeline-65b1e744defa\lib\rake-pipeline deleted the assets and tmp folder ran bundle exec rake build This is what I am using in my Assetfile:

concatFilter = Rake::Pipeline::ConcatFilter concatFilter.processes_binary_files match "static/*/" do filter concatFilter do |input| input.sub(/static//, '') end end

Note: I am sure your changes are being picked up because I get an error if I purposely corrupt "file_wrapper.rb"

— Reply to this email directly or view it on GitHub.

joefiorini avatar May 02 '13 03:05 joefiorini

Yes...

I am posting an original and after-corruption png... I am running on Windows 8

CORRUPTED

iphone4s_black

ORIGINAL

original-iphone4s_black

ronaldocpontes avatar May 02 '13 11:05 ronaldocpontes

If it helps here is the output from debugging the code:

FILE: img/theme/iphone4s_black.png before read : BINARY after read : BINARY contents.encoding : ASCII-8BIT forcing binary encoding... contents.encoding : ASCII-8BIT

FILE: app.js before read : BINARY after read : BINARY contents.encoding : ASCII-8BIT forcing binary encoding... contents.encoding : ASCII-8BIT

  def read
    print "\n\FILE: ",path
    print "\n before read      : ", encoding
    contents = if "".respond_to?(:encode)
      File.read(fullpath, :encoding => encoding)
    else
      File.read(fullpath)
    end
    print "\n after read        : ", encoding
    print "\n contents.encoding : ", contents.encoding

    # In our unit tests Rubinius returns false when the encoding is BINARY
    # The encoding type check bypasses the problem and is probably acceptable, but isn't ideal
    if encoding != "BINARY" && "".respond_to?(:encode) && !contents.valid_encoding?
      print "\n forcing binary encoding..."
      contents.force_encoding("BINARY")
      if !contents.valid_encoding?
        raise EncodingError, "The file at the path #{fullpath} is not valid #{encoding}. Please save it again as #{encoding}."
      end
    end
    if encoding == "BINARY" 
      print "\n forcing binary encoding..."
      contents.force_encoding("BINARY")
    end

    print "\n contents.encoding : ", contents.encoding
    contents
  end

ronaldocpontes avatar May 02 '13 17:05 ronaldocpontes

Strangely, the write method also seems to be on binary encoding...

FILE: img/theme/iphone4s_black.png before read : BINARY after read : BINARY contents.encoding : ASCII-8BIT forcing binary encoding... contents.encoding : ASCII-8BIT before write : ASCII-8BIT ->/assets/img/theme/iphone4s_black.png

FILE: app.css before read : BINARY after read : BINARY contents.encoding : ASCII-8BIT forcing binary encoding... contents.encoding : ASCII-8BIT before write : ASCII-8BIT ->assets/app.css

  def write(string)
    raise UnopenedFile unless @created_file
    print "\n before write      : ", string.encoding, " ->", @created_file.path
    @created_file.set_encoding string.encoding
    @created_file.write(string)
  end

ronaldocpontes avatar May 02 '13 18:05 ronaldocpontes

Hi Joe,

Just curious if you had a chance to replicate the issue...

Tks

ronaldocpontes avatar May 13 '13 20:05 ronaldocpontes

@ronaldocpontes yes, just tried it and it appears to be working for me. I'm going to try a slightly more detailed test, just in case.

joefiorini avatar May 14 '13 04:05 joefiorini

Just tried it with your exact setup. Ran the image through the same concat filter you posted above and it worked fine for me. My guess is it's a Windows issue. Does anyone else have a Windows setup they can use to try processing the above image on my fork?

joefiorini avatar May 14 '13 04:05 joefiorini

Hi Joe,

I finally could fix the issue on Windows with following set of methods: check: http://blog.leosoto.com/2008/03/reading-binary-file-on-ruby.html

  def read
    contents = if "".respond_to?(:encode) and encoding=="BINARY"
        contents = open(fullpath, "rb") {|io| io.read }
    else
      File.read(fullpath)
    end
    contents
  end


  def create
    FileUtils.mkdir_p(File.dirname(fullpath))

    @created_file = if "".respond_to?(:encode) and encoding=="BINARY"
      File.open(fullpath, "wb")
    else
      File.open(fullpath, "w")
    end

    if block_given?
      yield @created_file
    end

    @created_file
  ensure
    if block_given?
      @created_file.close
      @created_file = nil
    end
  end

  def write(string)
    raise UnopenedFile unless @created_file
    @created_file.set_encoding string.encoding
    @created_file.write(string)
  end

Could you test on your OS and update the pull request if you are happy with it?

Thanks

ronaldocpontes avatar May 14 '13 11:05 ronaldocpontes

Note that I removed some encoding manipulation, so for a non-binary file:

FILE: vendor/bootstrap.js encoding variable : UTF-8 contents.encoding : CP850

Write method: string.encoding : CP850

ronaldocpontes avatar May 14 '13 14:05 ronaldocpontes