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

Add symlink support, closes #8 #12

Closed
wants to merge 1 commit into from

Conversation

fetep
Copy link

@fetep fetep commented Mar 3, 2015

This creates tar archives with working symlinks, although I am having trouble making the test for this work. I end up with this as a failure, any pointers/help appreciated:

  1) Failure:
TestTarWriter#test_symlink [/nycasa.nydc.fxcorp.prv/nfs1/home/pfritchman/src/minitar/test/test_tar_writer.rb:178]:
Field name of the tar header differs..
--- expected
+++ actual
@@ -1 +1 @@
-"lib/foo\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"
+"lib/foo/bar\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000"

This change is Reviewable

@coveralls
Copy link

Coverage Status

Coverage increased (+0.5%) to 79.96% when pulling 09e991d on fetep:symlink_support into bfc5bfa on halostatue:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage increased (+0.5%) to 79.96% when pulling 09e991d on fetep:symlink_support into bfc5bfa on halostatue:master.

@halostatue
Copy link
Owner

I’ll have to look at this a bit more in depth—and I haven’t had a chance to do this so far, but the immediately problem that I’m seeing is that one of these is Unicode (\u0000) and the other is ASCII (\x00\x00).

@jordansissel
Copy link

@halostatue I've had problems like that before where Ruby's ideas of default string encoding is ... in disagreement with my assumptions.

Things maybe to check:

  • If you don't have an # encoding: utf-8 line at the top of your ruby files, I recommend this. I believe it's optional in Ruby 1.9.3, but assumed to be utf-8 in Ruby 2.0.0+.
  • If you are reading files, open them with binary mode (File.new("path", "rb"))
  • If you have explicit global settings, you can set Encoding.default_external = Encoding::UTF_8 and same for Encoding.default_internal.
  • If you know a string is a binary blob, you can force the encoding label in Ruby with String#force_encoding: `"foo".force_encoding("BINARY") to tell ruby that it's really a binary blob, not utf-8.

Hope this helps! I've had many encoding battles with Ruby in the past :P

@lollipopman
Copy link

@halostatue would love to see symlinks supported, any chance of this being worked on?

@halostatue
Copy link
Owner

@lollipopman I have no time to do so. The PR is essentially complete, except for the unit test failure, so you could start from there.

@halostatue halostatue mentioned this pull request Mar 26, 2022
@halostatue
Copy link
Owner

@fetep @lollipopman I think that I have this fixed properly in #42 against the current codebase. If you could test this, we can release it as part of 0.10 or shortly after as 0.11.

@halostatue halostatue closed this Mar 26, 2022
@lollipopman
Copy link

@halostatue awesome, I'll give it a try, thanks!

@halostatue
Copy link
Owner

@lollipopman Did you get a chance to test #12?

@lollipopman
Copy link

@halostatue, I tested it today, seems to work perfect, thanks again!

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

Successfully merging this pull request may close these issues.

None yet

5 participants