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

testing/etckeeper: new aport #788

Closed
wants to merge 1 commit into from
Closed

Conversation

HRio
Copy link
Contributor

@HRio HRio commented Jan 25, 2017

For limitations see README.alpine

@HRio
Copy link
Contributor Author

HRio commented Feb 1, 2017

Copy link
Contributor

@fabled fabled left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks mostly good. My biggest question is if the limitations, and hacky trigger script can be done after the apk-tools change? If yes, I'd like to do this right from the beginning and merge only after improved apk-tools. Thanks!

exec 200>"$LOCK"

# wait for $LOCK before commit
(flock --timeout 180 200 && etckeeper post-install) &
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this trigger script hack something we get rid of after alpinelinux/apk-tools#2 is merged?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes ;-) was not happy with that trigger at all, but it was the only way to do it with current apk-tools. Hence this trigger (removal of it) is the motivation for the work in alpinelinux/apk-tools#2 i.

Known Limitations
-----------------
* Can not block package installation
* Can not handle commit before install
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both are fixable after alpinelinux/apk-tools#2 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes thats right.

pkgname=etckeeper
pkgver=1.18.6
pkgrel=1
pkgdesc="Store /etc in git."
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I generally dislike this, because git tracks history, and in tmpfs installs it means the whole history is also stored in the apkovl. Which is part of the reason why we wrote aaudit. Then again, it's much less known, less feature complete etc. And I do understand that people are possibly familiar with etckeeper so I think we should include it as an option.

package() {
cd "$builddir"
sed -i 's|^PYTHON=python$|PYTHON=/bin/false|' Makefile || return 1
sed -i 's/_PACKAGE_MANAGER=.*/_PACKAGE_MANAGER=apk/' "$builddir"/etckeeper.conf || return 1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There sed scripts should probably be in prepare step.

@HRio
Copy link
Contributor Author

HRio commented Feb 15, 2017

Note 2720907 testing/etckeeper: use apk commit_hook
requires a new version of the apk-tools package for the commit hook to function

@kaniini
Copy link
Contributor

kaniini commented Feb 26, 2017

Can you flatten this down?

Note: uses a apk commit_hooks.d script, so it requires a new apk version.
@HRio
Copy link
Contributor Author

HRio commented Feb 26, 2017

@kaniini sure I can push a squash.

@kaniini
Copy link
Contributor

kaniini commented Feb 26, 2017

It seems documentation on how to use apk commit-hook is missing. Can you bring back the README.Alpine file for that?

@HRio
Copy link
Contributor Author

HRio commented Feb 26, 2017

The apk commit hook will not work before a new version of apk-tools has been tagged:
https://github.com/alpinelinux/apk-tools/commits/master
before that this PR can be kept on hold.

@kaniini
Copy link
Contributor

kaniini commented Feb 26, 2017

Okay. Lets hold on this one then.

@algitbot
Copy link

Merged in 188d8ba by @fabled. Thanks for your contribution!

(This pull request has been closed automatically by GitHub PR Closer. If you think that it’s not resolved yet, please add a comment.)

@algitbot algitbot closed this Feb 28, 2017
@HRio HRio deleted the etckeeper2 branch March 1, 2017 18:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants