terraspace build creates \r\n and invalid TF code on Windows

CumpsD commented 2 years ago

Checklist

[x] Upgrade Terraspace: Are you using the latest version of Terraspace? This allows Terraspace to fix issues fast. There's an Upgrading Guide: https://terraspace.cloud/docs/misc/upgrading/
[x] Reproducibility: Are you reporting a bug others will be able to reproduce and not asking a question. If you're unsure or want to ask a question, do so on https://community.boltops.com

My Environment

Software	Version
Operating System	Windows 11
Terraform	1.1.5
Terraspace	1.1.3

Expected Behaviour

terraspace build files exactly as they are

Current Behavior

Files using CRLF as line endings get formatted incorrectly, causing TF to fail.

Step-by-step reproduction instructions

I have this in my Terrafile:

mod "lambda", source: "terraform-aws-modules/lambda/aws"

One of the files looks like this in the vendor folder:

After running terraspace build it gets turned into this:

Which causes a lot of errors:

If I run dos2unix on the files, it works.

Solution Suggestion

Find out why Ruby does this to the files and avoid it if possible?

tongueroo commented 2 years ago

Wondering if this quick fix will do the trick. https://stackoverflow.com/questions/30195652/how-to-stop-ruby-automatically-adding-carriage-returns

Try throwing this at the top of:

lib/terraspace.rb

$\ = "\n"

And then running again

terraspace build

Thanks for the help testing

CumpsD commented 2 years ago

Sadly enough it didn't work :(

It only happens with *.tf files btw, readme.md and others are copied correctly.

CumpsD commented 2 years ago

Where is the actual file written? I can try this perhaps: https://stackoverflow.com/a/30213816/4329

tongueroo commented 2 years ago

File is written here. https://github.com/boltops-tools/terraspace/blob/37fdf43d02ccae63e60e6cc3a61afbe4b62bfb5f/lib/terraspace/compiler/writer.rb#L37 It’s written with IO.write though

CumpsD commented 2 years ago

    def write(content)
      FileUtils.mkdir_p(File.dirname(dest_path))
      if content.respond_to?(:path) # IO filehandle
        FileUtils.cp(content.path, dest_path) # preserves permission
      else # just content
        my_file = Tempfile.new("tmp.tf")
        my_file.binmode
        my_file.print content
        my_file.close
        FileUtils.mv(my_file.path, dest_path)

        # IO.write(dest_path, content)
      end
      logger.debug "Created #{Terraspace::Util.pretty_path(dest_path)}"
    end

This writes it out correctly :D

Now to play with it if IO.write has something similar

tongueroo commented 2 years ago

Sweet 🎊 Will take a look adding a write shim method that uses that for Windows and regular IO.write for other OSes. On the road right now. Feel free to have a go at a PR. No sweat either way. Hope to get to it. Probably over the weekend.

Note: The temp file name will have to be uniquely generated. As terraspace all can write from multiple processes and cause race conditions.

CumpsD commented 2 years ago

I'm trying to figure out how IO.write works :)

I found: https://ruby-doc.org/core-3.0.2/IO.html#method-c-new which says we can open for "wb" to write binary.

But when I try IO.write(dest_path, content, "wb") I got:

C:/Ruby31-x64/lib/ruby/gems/3.1.0/gems/terraspace-1.1.3/lib/terraspace/compiler/writer.rb:44:in `write': no implicit conversion from string (TypeError)
        from C:/Ruby31-x64/lib/ruby/gems/3.1.0/gems/terraspace-1.1.3/lib/terraspace/compiler/writer.rb:44:in `write'
        from C:/Ruby31-x64/lib/ruby/gems/3.1.0/gems/terraspace-1.1.3/lib/terraspace/compiler/perform.rb:61:in `compile_mod_file'
        from C:/Ruby31-x64/lib/ruby/gems/3.1.0/gems/terraspace-1.1.3/lib/terraspace/compiler/perform.rb:24:in `block in compile_module'
        from C:/Ruby31-x64/lib/ruby/gems/3.1.0/gems/terraspace-1.1.3/lib/terraspace/compiler/perform.rb:71:in `block in with_path'
        from C:/Ruby31-x64/lib/ruby/gems/3.1.0/gems/terraspace-1.1.3/lib/terraspace/compiler/perform.rb:69:in `each'
        from C:/Ruby31-x64/lib/ruby/gems/3.1.0/gems/terraspace-1.1.3/lib/terraspace/compiler/perform.rb:69:in `with_path'
        from C:/Ruby31-x64/lib/ruby/gems/3.1.0/gems/terraspace-1.1.3/lib/terraspace/compiler/perform.rb:65:in `with_mod_file'
        from C:/Ruby31-x64/lib/ruby/gems/3.1.0/gems/terraspace-1.1.3/lib/terraspace/compiler/perform.rb:23:in `compile_module'
        from C:/Ruby31-x64/lib/ruby/gems/3.1.0/gems/terraspace-1.1.3/lib/terraspace/compiler/perform.rb:12:in `compile'
        from C:/Ruby31-x64/lib/ruby/gems/3.1.0/gems/terraspace-1.1.3/lib/terraspace/builder.rb:66:in `block in build_dir'
        from C:/Ruby31-x64/lib/ruby/gems/3.1.0/gems/terraspace-1.1.3/lib/terraspace/compiler/dirs_concern.rb:18:in `block in with_each_mod'       
        from C:/Ruby31-x64/lib/ruby/gems/3.1.0/gems/terraspace-1.1.3/lib/terraspace/compiler/dirs_concern.rb:15:in `each'
        from C:/Ruby31-x64/lib/ruby/gems/3.1.0/gems/terraspace-1.1.3/lib/terraspace/compiler/dirs_concern.rb:15:in `with_each_mod'
        from C:/Ruby31-x64/lib/ruby/gems/3.1.0/gems/terraspace-1.1.3/lib/terraspace/builder.rb:63:in `build_dir'
        from C:/Ruby31-x64/lib/ruby/gems/3.1.0/gems/terraspace-1.1.3/lib/terraspace/builder.rb:39:in `block in build'
        from C:/Ruby31-x64/lib/ruby/gems/3.1.0/gems/terraspace-1.1.3/lib/terraspace/hooks/builder.rb:25:in `run_hooks'
        from C:/Ruby31-x64/lib/ruby/gems/3.1.0/gems/terraspace-1.1.3/lib/terraspace/hooks/concern.rb:6:in `run_hooks'
        from C:/Ruby31-x64/lib/ruby/gems/3.1.0/gems/terraspace-1.1.3/lib/terraspace/builder.rb:38:in `build'
        from C:/Ruby31-x64/lib/ruby/gems/3.1.0/gems/terraspace-1.1.3/lib/terraspace/builder.rb:25:in `run'
        from C:/Ruby31-x64/lib/ruby/gems/3.1.0/gems/terraspace-1.1.3/lib/terraspace/cli.rb:60:in `build'
        from C:/Ruby31-x64/lib/ruby/gems/3.1.0/gems/thor-1.2.1/lib/thor/command.rb:27:in `run'
        from C:/Ruby31-x64/lib/ruby/gems/3.1.0/gems/thor-1.2.1/lib/thor/invocation.rb:127:in `invoke_command'
        from C:/Ruby31-x64/lib/ruby/gems/3.1.0/gems/thor-1.2.1/lib/thor.rb:392:in `dispatch'
        from C:/Ruby31-x64/lib/ruby/gems/3.1.0/gems/terraspace-1.1.3/lib/terraspace/command.rb:61:in `dispatch'
        from C:/Ruby31-x64/lib/ruby/gems/3.1.0/gems/thor-1.2.1/lib/thor/base.rb:485:in `start'
        from C:/Ruby31-x64/lib/ruby/gems/3.1.0/gems/terraspace-1.1.3/lib/terraspace/cli/concern.rb:65:in `start'
        from C:/Ruby31-x64/lib/ruby/gems/3.1.0/gems/terraspace-1.1.3/exe/terraspace:14:in `<top (required)>'
        from C:/Ruby31-x64/bin/terraspace:25:in `load'
        from C:/Ruby31-x64/bin/terraspace:25:in `<main>'

So I am trying to find what's needed to use IO.write with wb as mode

CumpsD commented 2 years ago

I found https://ruby-doc.org/core-3.0.2/IO.html#method-c-write

If the last argument is a hash, it specifies options for the internal open(). It accepts the following keys:

:mode
string or integer

Specifies the mode argument for open(). It must start with “w”, “a”, or “r+”, otherwise it will cause an error. See [::new](https://ruby-doc.org/core-3.0.2/IO.html#method-c-new) for the list of possible modes.

Trying to get that to work now, I already know this is the wrong syntax: IO.write(dest_path, content, { mode: "wb" }) :D

Learning Ruby along the way

CumpsD commented 2 years ago

Got it!

        IO.write(dest_path, content, mode: "wb")

That's the fix needed for Windows, nothing more

ps, "binary" is a very confusing term in Ruby, it actually means this:

"b" Binary file mode Suppresses EOL <-> CRLF conversion on Windows. And sets external encoding to ASCII-8BIT unless explicitly specified.

CumpsD commented 2 years ago

Here you go: https://github.com/boltops-tools/terraspace/pull/210

boltops-tools / terraspace