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(BREAKING): ipfs daemon flags #1437

Merged
merged 1 commit into from
Apr 23, 2020
Merged

fix(BREAKING): ipfs daemon flags #1437

merged 1 commit into from
Apr 23, 2020

Conversation

hacdias
Copy link
Member

@hacdias hacdias commented Apr 23, 2020

Fixes the flags we pass to the ipfs daemon. After merging #1392, my daemon wouldn't start because the --migrate flag was malformed. We don't need the =true. I looked at ipfsd-ctl tests and it seems the flags now should be set like this. Without migrating, the boot would hang.

It may be breaking for users that changed the flags by themselves. For simplicty, I just replaced the flags in the configuration to:

[
  '--migrate',
  '--enable-gc',
  '--routing', 'dhtclient'
]

This can be added as a breaking change on the release notes. Probably only some power users know about this and changed it so they'll know what to do once they see this.

License: MIT
Signed-off-by: Henrique Dias hacdias@gmail.com

@hacdias hacdias requested a review from lidel April 23, 2020 07:57
@hacdias hacdias mentioned this pull request Apr 23, 2020
22 tasks
@hacdias hacdias added kind/bug A bug in existing code (including security flaws) P0 Critical: Tackled by core team ASAP labels Apr 23, 2020
Copy link
Member

@lidel lidel left a comment

Choose a reason for hiding this comment

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

What if we run fixup only when ipfsConfig.flags list has --migrate=true or --enable-gc=true present? Should be much safer.

For everything else, just change the defaults in

https://github.com/ipfs-shipyard/ipfs-desktop/blob/0ee2ac001cf0d846144c786afa2cec48ba45daf4/src/common/store.js#L13

License: MIT
Signed-off-by: Henrique Dias <hacdias@gmail.com>
@hacdias
Copy link
Member Author

hacdias commented Apr 23, 2020

@lidel just changed. Now we just update if those flags are already there.

@hacdias hacdias requested a review from lidel April 23, 2020 11:26
Copy link
Member

@lidel lidel left a comment

Choose a reason for hiding this comment

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

LGTM, merge at your convenience!

@hacdias hacdias merged commit 1f42140 into master Apr 23, 2020
@hacdias hacdias deleted the fix/flags branch April 23, 2020 12:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug A bug in existing code (including security flaws) P0 Critical: Tackled by core team ASAP
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants