Skip to content

/ makeself

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

Jump to bottom

Append appears to be broken #216

Closed
andyneff opened this issue Jun 18, 2020 · 13 comments
Closed

Append appears to be broken #216

andyneff opened this issue Jun 18, 2020 · 13 comments

Comments

@andyneff
Copy link
Contributor

@andyneff andyneff commented Jun 18, 2020

I'm running master 39da42d and --append does not appear to be working. (Also tried 2.4.2)

Steps to reproduce

  1. I create an archive makeself.sh ./somedir archive.sh label_me ./myscript
  2. I append to it makeself.sh --append ./new_files archive.sh

Error observed

When the second makeself.sh call is made, I see this on the output:

About to compress 88 KB of data...
Adding files to archive named "./dist/just"...
tail -n + ./dist/just
tail: invalid number of lines: ‘+’

gzip: stdin: unexpected end of file

Which comes from this line, where OLDSKIP is undefined.

I can see in the history, that OLDSKIP used to be SKIP+1. but it was removed recently, but that was only in dumpconf, so I'm not sure how to fix this.

I tried adding SKIP+1 myself, but sometimes its +1, sometimes its +2 (verified in vi)

Tests

The unittest appendtest actually has the same problem

Adding files to archive named "makeself-test.run"...
tail: invalid number of lines: ‘+’

gzip: stdin: unexpected end of file

Checking --check doesn't actually detect that the previous files were all deleted. Some for the other compression algorithms

Desired outcome

Append should work
@megastep
Copy link
Owner

@megastep megastep commented Jun 18, 2020

Odd - what OS are you on?
@andyneff
Copy link
Contributor Author

@andyneff andyneff commented Jun 18, 2020

I'm running Fedora 31, but I can try another OS in docker if you have one I should check out?
@andyneff
Copy link
Contributor Author

@andyneff andyneff commented Jun 18, 2020

  • About the +1 or +2 comment, the value of skip in the header of the archive + 1 is consistently the right line number.
@megastep
Copy link
Owner

@megastep megastep commented Jun 18, 2020

Right, that's what it should be. I'm just wondering it there's something about your environment that would make this line count disappear somehow.
@andyneff
Copy link
Contributor Author

@andyneff andyneff commented Jun 18, 2020

I can see no where in the source code where OLDSKIP is set
@megastep
Copy link
Owner

@megastep megastep commented Jun 18, 2020

Yeah I'm seeing that too. That sounds like the cause of this, checking when that variable went away...
@andyneff
Copy link
Contributor Author

@andyneff andyneff commented Jun 18, 2020

According to my debugger, I think the value of skip we need is stored in OLDENV?

This was the commit I found there it was mentioned. Not sure how they were connected: 0d093c0
@andyneff
Copy link
Contributor Author

@andyneff andyneff commented Jun 18, 2020 •
edited

Oh, I see. This line used to set the OLDSKIP that used to be part of --dumpconf
@megastep
Copy link
Owner

@megastep megastep commented Jun 18, 2020

Also our unit tests for append are obviously broken :(
@andyneff
Copy link
Contributor Author

@andyneff andyneff commented Jun 18, 2020

I think they are testing the archive is "good", but not that they have all the files. The archive is valid, so I'd just call them "not sufficient" :)
@andyneff
Copy link
Contributor Author

@andyneff andyneff commented Jun 18, 2020 •
edited

Adding

OLDSKIP=`expr $SKIP + 1`

After this line works
@megastep
Copy link
Owner

@megastep megastep commented Jun 18, 2020

It does work but it seems like instead of appending it is just overwriting the archive still, losing the previous contents.
@andyneff
Copy link
Contributor Author

@andyneff andyneff commented Jun 18, 2020 •
edited

Hmmm, that fix successfully appends for me...
@andyneff andyneff mentioned this issue Jun 18, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Linked pull requests

Successfully merging a pull request may close this issue.

Fix append
2 participants
@megastep @andyneff