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

Fix stuff and add Podman/Fedora support (#986 rebase) #1133

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

penn5
Copy link

@penn5 penn5 commented Jun 20, 2021

No description provided.

@spantaleev
Copy link
Owner

This PR is huge and changes lots of unrelated things. A lot of these changes are dubious:

  • docker stop instead of docker kill. I guess you did it because docker kill fails on Podman, but it may be more unreliable on Docker (untested)
  • enabling Ansible retry files, which we'd rather not have
  • unrelated changes to mautrix-signal in 3339186
  • various other unrelated changes in e8047bc

It'd be great to add support for Fedora and Podman (on Fedora or even on other systems), but these changes should be done in an isolated manner, without introducing a ton of other changes across the whole codebase.

@penn5
Copy link
Author

penn5 commented Jun 21, 2021

I'd be happy for you to pick individual commits from this PR as they get reviewed, if that makes things simpler. They would still need to be applied in the order given (though the retry files is not required, I just needed it for testing)

@penn5
Copy link
Author

penn5 commented Jun 21, 2021

e8047bc fixes ARM support
3339186 makes setting up mautrix-signal simpler but again obviously not needed for Fedora.
docker kill does work on podman (iirc, it's been a while), but leads to database corruption (well, it's recoverable, but still not ideal) because it sends SIGKILL instead of SIGINT. docker stop will send first SIGINT (or whatever signal is needed, SIGTERM or whatever) and wait some timeout before sending SIGKILL. So hopefully it has no risk of breaking things. It all worked for me.

@ttuffin
Copy link

ttuffin commented Feb 23, 2022

Is this PR still under consideration? @spantaleev would you consider opening up a new branch for the development of this feature?

@spantaleev
Copy link
Owner

You can fork this project and make your own branch there.

This PR is very outdated now anyway and likely not a great way to approach things nowadays.

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

Successfully merging this pull request may close these issues.

3 participants