Added libsecret

[ManiacTwister]

As mentioned in the other pull request ( #825 ) libsecret is the replacement for gnome-keyring, so here is the new implementation.

cheers

[Arnavion]

Apart from the commit message typo ("secure" instead of "secret" - this can be fixed when we merge it) and the whitespace issues, I think this is fine.

[TingPing]

• Use COMMON_CFLAGS not CPPFLAGS. (same for LIBS)
• Default to yes not auto, and set to no on failure.
• Use libsecret instead of use_libsecret to match other vars.

[ManiacTwister]

Just found a bug:
If heschats exits immediately after servlist_save were called ( = if no sessions are open) any changed password will not be updated because the libsecret thread will not finish fast enough. I have two ideas how to fix this:

• Add an argument "shutdown" to servlist_save. When shutdown is true use secret_password_store_sync (= blocking) instead of secret_password_store. In servlistgui.c check if hexchat will close after saveing and if yes call servlist_save with shutdown=true.
• Add the same argument as above but use secret_password_store and close hexchat in the libsecure "finished-callback" and not in servlistgui_save.
• Use always secret_password_store_sync instead of secret_password_store. With the default serverlist ( ~ 90 servers) the servlistgui blocks for approximately 1,5seconds on my pc everytimei close it.

Any suggestions?

[TingPing]

@ManiacTwister Probably the first option except check hexchat_is_quitting variable instead of adding an arg.

On an unrelated note it doesn't seem you check at all if the storing succeeds (or is supported at least)? Not every DE will have a keyring and it needs to fallback to the config file.

[ManiacTwister]

@TingPing hexchat_is_quitting is set to true AFTER servlist_save is called. Should i set it to true manually in servlist_savegui?

Error handling isn't really possible when using secret_password_store because the result will be available in the callback only which will be called after the servist is written..

[TingPing]

Error handling isn't really possible when using secret_password_store because the result will be available in the callback only which will be called after the servlist is written..

Well you have to find a way or disable this feature by default making it nearly useless.
I guess in the callback just detect the error, set that its not supported, and rewrite it then all saves after will just skip it.

Should i set it to true manually in servlist_savegui?

How would servlist know we are quitting? You can just call servlist_save once in hexchat_exit.

[ManiacTwister]

Well you have to find a way or disable this feature by default making it nearly useless.

I'll look a bit depper into libsecret, maybe there is a elegant way to do this.

How would servlist know we are quitting? You can just call servlist_save once in hexchat_quit.

servlist_close_cb (GtkWidget *button, gpointer userdata)
{
    servlist_savegui ();
    gtk_widget_destroy (serverlist_win);
 [....]
    if (sess_list == NULL)
        hexchat_exit ();
}

As far as i understand the close-button callback, it is responsible for exiting hexchat if no sessions are open, so this should work.

[TingPing]

As far as i understand the close-button callback, it is responsible for exiting hexchat if no sessions are open, so this should work.

I see, setting it there should be fine then.

[ManiacTwister]

I have added an fallback now but i don't know if it it's really clean to do it like this. If you agree with the changes in the new branch [1] i'll merge it whit the pull request

[1] ManiacTwister@01f5b87

[TingPing]

@ManiacTwister Seems to work fine but I would say if it fails once it should just set a preference to disable the feature entirely (also giving users the option to disable it manually) not retry it multiple times every time.

[ManiacTwister]

@TingPing Added config option: ManiacTwister/hexchat@01f5b87...e787198

[TingPing]

@ManiacTwister I like it other than the setup page, it is just empty and makes no more sense under Network than in the advanced page.

[ManiacTwister]

@TingPing Done and merged the changes, but i'm not sure if the libsecret setting should be moved up to the other checkbox-settings. (maybe between hex_irc_cap_server_time and hex_net_auto_reconnect ?)

[tomek]

@ManiacTwister before anything - rebase it against our master as github itself shows there are some conflicts - and you could reduce numer of commits.

[ManiacTwister]

@tomek @TingPing So.. any news about this one? I'm using this patch locally since 7 months without problems.

[leeter]

odd I didn't think this pull was that old... perhaps the copyright should be updated

[ManiacTwister]

Happy 3. Birthday to my pullrequest

[TingPing]

@ManiacTwister So what I really want to happen is some abstract interface for storing passwords with backends for OSX, Windows, libsecret, plaintext, etc. This PR, while perhaps functional, is sort of an ugly mess of ifdefs throughout the code base that is not easily extended. Also I don't think you ever handled migration?

[DerekTurtleRoe]

@ManiacTwister @TingPing This seems like a cool idea, has anybody looked at it recently?