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 SafeWriteConfig #450

Merged
merged 1 commit into from
Jul 28, 2019
Merged

Fix SafeWriteConfig #450

merged 1 commit into from
Jul 28, 2019

Conversation

rchiossi
Copy link
Contributor

If the config file does not exist and the force flag is not set,
OpenFile would not use O_CREATE flag, causing viper to fail with
error "File not exist" and to not create the config file.
This patch changes the behavior of writeConfig() so that if force is set
to false, OpenFile will use O_EXCL flag, thus failing if the file
already exists or creating a new file otherwise.

Signed-off-by: Rodrigo Chiossi rodrigo.chiossi@intel.com

If the config file does not exist and the force flag is not set,
OpenFile would not use O_CREATE flag, causing viper to fail with
error "File not exist" and to not create the config file.
This patch changes the behavior of writeConfig() so that if force is set
to false, OpenFile will use O_EXCL flag, thus failing if the file
already exists or creating a new file otherwise.

Signed-off-by: Rodrigo Chiossi <rodrigo.chiossi@intel.com>
@CLAassistant
Copy link

CLAassistant commented Jan 30, 2018

CLA assistant check
All committers have signed the CLA.

@Nhoya
Copy link

Nhoya commented Apr 1, 2018

Can someone please review this pull request?

@Vaelatern
Copy link

Vaelatern commented Feb 5, 2019

This patch works for me, and the default behavior is indeed broken.
I'd like to use this functionality please.
@spf13 I'm sorry to ping you directly, but it would really be nice to have this merged.

@rchiossi
Copy link
Contributor Author

rchiossi commented Feb 6, 2019

Given that this is a very small PR and that it is waiting for review for over one year, I believe this project has sadly been abandoned.

@Vaelatern
Copy link

The latest commit is just 12 days ago, so the project seems somewhat alive. That this doesn't work as documented is disheartening.

@sagikazarmark
Copy link
Collaborator

Ping one of the frequent commiter (bep usually responds, but I don't want to ping him).

@Vaelatern
Copy link

@bep I'm sorry for pinging you directly, would you be able to merge this PR so as to fix this function?

@estroz
Copy link

estroz commented May 1, 2019

@bep @spf13 can you check this out/merge?

@Vaelatern
Copy link

Vaelatern commented Jul 28, 2019

@sagikazarmark Could you possibly merge this when you have a chance? I've noticed you merging things recently, and know that bep and spf13 are not currently responding.

@bep bep merged commit cdccc81 into spf13:master Jul 28, 2019
@rchiossi
Copy link
Contributor Author

Finally! \o/
Too bad I already moved away from Viper. Still, I'm happy to see someone is picking up on its development.

@Vaelatern
Copy link

Celebration!

@Pokerkoffer
Copy link

So when can we expect a new release containing this PR?

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.

8 participants