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

Fix order of tar args #3895

Merged
merged 3 commits into from Apr 19, 2018

Conversation

Projects
None yet
2 participants
@NiLuJe
Member

NiLuJe commented Apr 18, 2018

Otherwise, current tar versions abort

NiLuJe added some commits Apr 18, 2018

Fix order of tar args
Otherwise, current tar versions abort
Unbreak Kobo IR grid resume workaround
Regression after f402ee5

I'm going to assume my H2O is really, really weird, because that
commit's more than a year old o_O.
@NiLuJe

This comment has been minimized.

Member

NiLuJe commented Apr 18, 2018

Sneaked in a fix for an (old) regression for a (weird) bug.

Makefile Outdated
@@ -186,8 +186,8 @@ kindleupdate: all
# note that the targz file extension is intended to keep ISP from caching
# the file, see koreader#1644.
cd $(INSTALL_DIR) && \
tar czafh ../koreader-$(DIST)-$(MACHINE)-$(VERSION).targz \
-T koreader/ota/package.index --no-recursion
tar --no-recursion -czahf ../koreader-$(DIST)-$(MACHINE)-$(VERSION).targz \

This comment has been minimized.

@Frenzie

Frenzie Apr 19, 2018

Member

Might be clearer to write -czahf --no-recursion instead?

This comment has been minimized.

@NiLuJe

NiLuJe Apr 19, 2018

Member

I have a tendency to like having the tarball filename directly following the 'f' arg, but maybe that's just me?

This comment has been minimized.

@NiLuJe

NiLuJe Apr 19, 2018

Member

Noope, that's not just me, that's tar itself ^^.

It'll try to create a tarball named --no-recursion and then fall on its ass because there's no koreader-<blah>.tar.gz to add to it. So, unwanted behavior on both fronts ;).

This comment has been minimized.

@NiLuJe

NiLuJe Apr 19, 2018

Member

That's with tar 1.30, FWIW.

This comment has been minimized.

@Frenzie

Frenzie Apr 19, 2018

Member

Sure, good point. But, and this is just my opinion, I think in that case it's clearer to write something like -czah --no-recursion -f <filename>.

This comment has been minimized.

@NiLuJe

NiLuJe Apr 19, 2018

Member

Appears to work, and I have no objection/preference, so, let's :).

Reorder tar args
Following suggestion

@Frenzie Frenzie merged commit 95bc5d7 into koreader:master Apr 19, 2018

1 check passed

ci/circleci Your tests passed on CircleCI!
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment