Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure files are read with the proper encoding #112

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

joefiorini
Copy link

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

@ronaldocpontes
Copy link

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
Copy link

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"

@joefiorini
Copy link
Author

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.

@ronaldocpontes
Copy link

Yes...

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

CORRUPTED

iphone4s_black

ORIGINAL

original-iphone4s_black

@ronaldocpontes
Copy link

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
Copy link

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
Copy link

Hi Joe,

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

Tks

@joefiorini
Copy link
Author

@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
Copy link
Author

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?

@ronaldocpontes
Copy link

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
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants