Author Topic: New switch --user-config  (Read 14052 times)

Offline dmoore

  • Developer
  • Lives here!
  • *****
  • Posts: 1576
New switch --user-config
« on: January 20, 2014, 03:33:58 pm »
Attached is a patch that adds support for specifying the user data directory:

Code
codeblocks --user-config=/home/myusername/.codeblocks-XXX

will set the user-config in /home/myusername/.codeblocks-XXX instead of the default (on Linux) of /home/myusername/.codeblocks

Useful for when you have multiple C::B versions that you want to keep totally isolated. (E.g. they might have different, incompatible user plugins installed)

Not thoroughly tested yet so there will be updates. (Also probably need to do a check to make sure the destination is writeable and abort if not.)

PS: Wasn't sure whether to call this user-config or user-data. wx confusingly refers to user-config as the home folder. Thoughts?

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5916
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Re: New switch --user-config
« Reply #1 on: January 20, 2014, 03:44:50 pm »
...
Useful for when you have multiple C::B versions that you want to keep totally isolated. (E.g. they might have different, incompatible user plugins installed)
...
Why not use the -p option which specify a different user profile file? (I think using different user profile is enough, no need to specify a different user profile directory, but I may be wrong)
If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.

Offline dmoore

  • Developer
  • Lives here!
  • *****
  • Posts: 1576
Re: New switch --user-config
« Reply #2 on: January 20, 2014, 03:51:57 pm »
Plugins and other crap in the user data dir will cause some very surprising results if you run an incompatible CB version! (Crashes,  completely borked UI etc). profiles don't take care of this.
« Last Edit: January 20, 2014, 05:22:30 pm by dmoore »

Offline MortenMacFly

  • Administrator
  • Lives here!
  • *****
  • Posts: 9694
Re: New switch --user-config
« Reply #3 on: January 20, 2014, 06:11:43 pm »
...nice idea, indeed.
Compiler logging: Settings->Compiler & Debugger->tab "Other"->Compiler logging="Full command line"
C::B Manual: https://www.codeblocks.org/docs/main_codeblocks_en.html
C::B FAQ: https://wiki.codeblocks.org/index.php?title=FAQ

Offline MortenMacFly

  • Administrator
  • Lives here!
  • *****
  • Posts: 9694
Re: New switch --user-config
« Reply #4 on: January 21, 2014, 08:01:57 am »
Thoughts?
I had a quick look into. Is there a reason why you bind this functionality on CfgMgrBldr instead of ConfigManager as it is done with all the other path's? Its weird to find this method in the builder pattern... (see, how --prefix is handled...)
Compiler logging: Settings->Compiler & Debugger->tab "Other"->Compiler logging="Full command line"
C::B Manual: https://www.codeblocks.org/docs/main_codeblocks_en.html
C::B FAQ: https://wiki.codeblocks.org/index.php?title=FAQ

Offline dmoore

  • Developer
  • Lives here!
  • *****
  • Posts: 1576
Re: New switch --user-config
« Reply #5 on: January 21, 2014, 01:25:43 pm »
I started exactly as you suggest, by copying prefix more or less. But the problem is that CfgMgrBldr is constructed as a side effect of the first manager::get so using configmanager::write to set it from the main App (as is done for prefix) is too late.

These are static functions/members so I guess I can move them back to ConfigManager,  but CfgMgrBldr will still need to access them. What do you think?

Also, will probably rename the switch --user data dir since it applies to config files and other user data.

Offline dmoore

  • Developer
  • Lives here!
  • *****
  • Posts: 1576
Re: New switch --user-config
« Reply #6 on: January 21, 2014, 08:28:09 pm »
New version of patch...


1. Switch is now called --user-data-dir

Code
codeblocks --user-data-dir=/my/writeable/path

2. The new methods and members are now part of ConfigManager. The members are private, the methods are protected to prevent them being touched by other parts of the source or plugins. For this to work I had to add CodeblocksApp as a friend class of ConfigManager.

TODO: Could make sure directory is writeable and quit if not.

Code
Index: src/include/configmanager.h
===================================================================
--- src/include/configmanager.h (revision 9594)
+++ src/include/configmanager.h (working copy)
@@ -87,6 +87,7 @@
 };
 
 
+class CodeBlocksApp;
 
 /* ------------------------------------------------------------------------------------------------------------------
 *  ConfigManager class
@@ -94,6 +95,7 @@
 class DLLIMPORT ConfigManager
 {
     friend class CfgMgrBldr;
+    friend class CodeBlocksApp;
 
     TiXmlDocument *doc;
     TiXmlElement* root;
@@ -106,18 +108,30 @@
     inline void Collapse(wxString& str) const;
     wxString InvalidNameMessage(const wxString& what, const wxString& sub, TiXmlElement *localPath) const;
     static void InitPaths();
+    static inline wxString GetUserConfigDir();
 
     static wxString config_folder;
     static wxString home_folder;
     static wxString data_path_user;
     static wxString data_path_global;
+
 #ifdef CB_AUTOCONF
     static wxString plugin_path_global;
 #endif
     static wxString app_path;
     static wxString temp_folder;
     static bool relo;
+    static wxString alternate_user_data_path;
+    static bool has_alternate_user_data_path;
 
+protected:
+    //For use by the CodeBlocksApp when the --user-data-dir switch is set
+    //all of the user config and user plugin data will be set relative to this path
+    static void SetUserDataFolder(const wxString &user_data_path);
+
+    //Used by CfgMgrBldr internally by ConfigManager
+    static wxString GetUserDataFolder();
+
 public:
 
     /* -----------------------------------------------------------------------------------------------------
Index: src/sdk/configmanager.cpp
===================================================================
--- src/sdk/configmanager.cpp (revision 9594)
+++ src/sdk/configmanager.cpp (working copy)
@@ -53,6 +53,9 @@
 template<> CfgMgrBldr* Mgr<CfgMgrBldr>::instance = nullptr;
 template<> bool  Mgr<CfgMgrBldr>::isShutdown = false;
 
+wxString ConfigManager::alternate_user_data_path;
+bool ConfigManager::has_alternate_user_data_path=false;
+
 wxString ConfigManager::config_folder;
 wxString ConfigManager::home_folder;
 wxString ConfigManager::data_path_user;
@@ -64,6 +67,8 @@
 wxString ConfigManager::temp_folder;
 bool ConfigManager::relo = 0;
 
+
+
 #ifdef __WINDOWS__
 inline wxString GetPortableConfigDir()
 {
@@ -71,7 +76,7 @@
     if (::GetEnvironmentVariable(_T("APPDATA"), buffer, MAX_PATH))
         return wxString::Format(_T("%s\\CodeBlocks"), buffer);
     else
-        return wxStandardPathsBase::Get().GetUserDataDir();
+        return ConfigManager::GetUserDataFolder();
 }
 #endif
 
@@ -194,7 +199,7 @@
         #ifdef __WINDOWS__
         cfg = GetPortableConfigDir() + wxFILE_SEP_PATH + personality + _T(".conf");
         #else
-        cfg = wxStandardPathsBase::Get().GetUserDataDir() + wxFILE_SEP_PATH + personality + _T(".conf");
+        cfg = ConfigManager::GetUserDataFolder() + wxFILE_SEP_PATH + personality + _T(".conf");
         #endif
         doc = new TiXmlDocument();
         doc->InsertEndChild(TiXmlDeclaration("1.0", "UTF-8", "yes"));
@@ -213,7 +218,7 @@
 #ifdef __WINDOWS__
     wxString u(GetPortableConfigDir() + wxFILE_SEP_PATH + filename);
 #else
-    wxString u(wxStandardPathsBase::Get().GetUserDataDir() + wxFILE_SEP_PATH + filename);
+    wxString u(ConfigManager::GetUserDataFolder() + wxFILE_SEP_PATH + filename);
 #endif
     wxString e(::DetermineExecutablePath() + wxFILE_SEP_PATH + filename);
 
@@ -439,7 +444,6 @@
     return c;
 }
 
-
 /*
 *  Hack to enable Turkish language. wxString::Upper will convert lowercase 'i' to \u0130 instead of \u0069 in Turkish locale,
 *  which will break the config file when used in a tag
@@ -552,6 +556,20 @@
     }
 }
 
+inline wxString ConfigManager::GetUserDataFolder()
+{
+    if (has_alternate_user_data_path)
+        return alternate_user_data_path;
+    return wxStandardPathsBase::Get().GetUserDataDir();
+}
+
+void ConfigManager::SetUserDataFolder(const wxString &user_data_path)
+{
+    //TODO: Check that this is a valid writeable path
+    has_alternate_user_data_path = true;
+    ConfigManager::alternate_user_data_path = user_data_path;
+}
+
 wxString ConfigManager::LocateDataFile(const wxString& filename, int search_dirs)
 {
     wxPathList searchPaths;
@@ -1440,7 +1458,7 @@
 #ifdef __WINDOWS__
     ConfigManager::config_folder = GetPortableConfigDir();
 #else
-    ConfigManager::config_folder = wxStandardPathsBase::Get().GetUserDataDir();
+    ConfigManager::config_folder = ConfigManager::GetUserDataFolder();
 #endif
     ConfigManager::home_folder = wxStandardPathsBase::Get().GetUserConfigDir();
     ConfigManager::app_path = ::DetermineExecutablePath();
Index: src/src/app.cpp
===================================================================
--- src/src/app.cpp (revision 9594)
+++ src/src/app.cpp (working copy)
@@ -212,6 +212,8 @@
       wxCMD_LINE_VAL_NONE, wxCMD_LINE_PARAM_OPTIONAL },
     { wxCMD_LINE_OPTION, CMD_ENTRY(""),   CMD_ENTRY("prefix"),                CMD_ENTRY("the shared data dir prefix"),
       wxCMD_LINE_VAL_STRING, wxCMD_LINE_NEEDS_SEPARATOR },
+    { wxCMD_LINE_OPTION, CMD_ENTRY(""),   CMD_ENTRY("user-data-dir"),                CMD_ENTRY("set a custom location for user configuration files"),
+      wxCMD_LINE_VAL_STRING, wxCMD_LINE_NEEDS_SEPARATOR },
     { wxCMD_LINE_OPTION, CMD_ENTRY("p"),  CMD_ENTRY("personality"),           CMD_ENTRY("the personality to use: \"ask\" or <personality-name>"),
       wxCMD_LINE_VAL_STRING, wxCMD_LINE_NEEDS_SEPARATOR },
     { wxCMD_LINE_SWITCH, CMD_ENTRY(""),   CMD_ENTRY("no-log"),                CMD_ENTRY("turn off the application log"),
@@ -313,6 +315,11 @@
     if (ParseCmdLine(nullptr) == -1) // only abort if '--help' was passed in the command line
         return false;
 
+    if (m_UserDataDir!=wxEmptyString)
+        ConfigManager::SetUserDataFolder(m_UserDataDir); // if --user-data-dir=path was specified we tell
+                                                     //ConfigManager (and CfgMgrBldr) about it, which will propagate
+                                                     //it through config manager
+
     ConfigManager *cfg = Manager::Get()->GetConfigManager(_T("app"));
 
     wxString data(wxT(APP_PREFIX));
@@ -1100,6 +1107,7 @@
                 {
                     wxString val;
                     parser.Found(_T("prefix"), &m_Prefix);
+                    parser.Found(_T("user-data-dir"), &m_UserDataDir);
 #ifdef __WXMSW__
                     m_DDE = !parser.Found(_T("no-dde"));
                     m_Assocs = !parser.Found(_T("no-check-associations"));
Index: src/src/app.h
===================================================================
--- src/src/app.h (revision 9594)
+++ src/src/app.h (working copy)
@@ -69,7 +69,8 @@
         void SetupPersonality(const wxString& personality);
 
 
-        wxString m_Prefix; // --prefix switch
+        wxString m_Prefix; // directory specified in --prefix switch
+        wxString m_UserDataDir; // directory specified in --user-data-dir switch
         wxString m_BatchTarget;
         wxString m_Script;
         wxString m_AutoFile; // --file foo.cpp[:line]


[attachment deleted by admin]

Offline oBFusCATed

  • Developer
  • Lives here!
  • *****
  • Posts: 13413
    • Travis build status
Re: New switch --user-config
« Reply #7 on: January 21, 2014, 08:58:19 pm »
For this to work I had to add CodeblocksApp as a friend class of ConfigManager.
This sounds bad. As far as I know the ConfigManager is in libcodeblocks and CodeblocksApp is in codeblocks.exe, seems dangerous to me.
Why don't you just stick a big comment above them, explicitly stating that they should be called only by the main application linking libcodeblocks.
Keep in mind that it is possible to have multiple codeblocks.exe versions. For example Jens has a branch where there are two such executables.
(most of the time I ignore long posts)
[strangers don't send me private messages, I'll ignore them; post a topic in the forum, but first read the rules!]

Offline dmoore

  • Developer
  • Lives here!
  • *****
  • Posts: 1576
Re: New switch --user-config
« Reply #8 on: January 21, 2014, 09:46:46 pm »
For this to work I had to add CodeblocksApp as a friend class of ConfigManager.
This sounds bad. As far as I know the ConfigManager is in libcodeblocks and CodeblocksApp is in codeblocks.exe, seems dangerous to me.
Why don't you just stick a big comment above them, explicitly stating that they should be called only by the main application linking libcodeblocks.
Keep in mind that it is possible to have multiple codeblocks.exe versions. For example Jens has a branch where there are two such executables.

Making them public kind of defeats the purpose of all this builder/singleton pattern nonsense in the first place. But really, I don't care, I just want the feature. (Btw, friend class doesn't require the class actually exists right? It just prevents non-friends from using the methods.)
« Last Edit: January 22, 2014, 04:10:13 am by dmoore »

Offline oBFusCATed

  • Developer
  • Lives here!
  • *****
  • Posts: 13413
    • Travis build status
Re: New switch --user-config
« Reply #9 on: January 21, 2014, 10:14:48 pm »
I think you need to have it forward declared.
(most of the time I ignore long posts)
[strangers don't send me private messages, I'll ignore them; post a topic in the forum, but first read the rules!]

Offline dmoore

  • Developer
  • Lives here!
  • *****
  • Posts: 1576
Re: New switch --user-config
« Reply #10 on: January 21, 2014, 10:16:38 pm »
New patch. This version does a simple check for the existence of the user-data-dir, creates if not, and errors out if that doesn't work.

I haven't yet made the protected members public and removed the friend class, but that's trivial if that's what we want to do.

Offline dmoore

  • Developer
  • Lives here!
  • *****
  • Posts: 1576
Re: New switch --user-config
« Reply #11 on: January 21, 2014, 10:19:06 pm »
I think you need to have it forward declared.

That's what I did, right? (In ConfigManager.h)

Offline osdt

  • Multiple posting newcomer
  • *
  • Posts: 63
Re: New switch --user-config
« Reply #12 on: January 22, 2014, 12:38:43 am »
New patch. This version does a simple check for the existence of the user-data-dir, creates if not, and errors out if that doesn't work. ...
The patch doesn't work on Windows because of GetPortableConfigDir(). It is called 3 times and would always return $APPDATA/CodeBlocks on most Systems.

The patch below (on top of yours) works on Windows and Linux.

Code
diff --git src/sdk/configmanager.cpp src/sdk/configmanager.cpp
index 4f569a6..b7a30ac 100644
--- src/sdk/configmanager.cpp
+++ src/sdk/configmanager.cpp
@@ -68,18 +68,6 @@ wxString ConfigManager::temp_folder;
 bool ConfigManager::relo = 0;
 
 
-
-#ifdef __WINDOWS__
-inline wxString GetPortableConfigDir()
-{
-    TCHAR buffer[MAX_PATH];
-    if (::GetEnvironmentVariable(_T("APPDATA"), buffer, MAX_PATH))
-        return wxString::Format(_T("%s\\CodeBlocks"), buffer);
-    else
-        return ConfigManager::GetUserDataFolder();
-}
-#endif
-
 namespace CfgMgrConsts
 {
     const wxString app_path(_T("app_path"));
@@ -196,11 +184,8 @@ CfgMgrBldr::CfgMgrBldr() : doc(nullptr), volatile_doc(nullptr), r(false)
 
     if (cfg.IsEmpty())
     {
-        #ifdef __WINDOWS__
-        cfg = GetPortableConfigDir() + wxFILE_SEP_PATH + personality + _T(".conf");
-        #else
         cfg = ConfigManager::GetUserDataFolder() + wxFILE_SEP_PATH + personality + _T(".conf");
-        #endif
+
         doc = new TiXmlDocument();
         doc->InsertEndChild(TiXmlDeclaration("1.0", "UTF-8", "yes"));
         doc->InsertEndChild(TiXmlElement("CodeBlocksConfig"));
@@ -215,11 +200,7 @@ wxString CfgMgrBldr::FindConfigFile(const wxString& filename)
 {
     wxPathList searchPaths;
 
-#ifdef __WINDOWS__
-    wxString u(GetPortableConfigDir() + wxFILE_SEP_PATH + filename);
-#else
     wxString u(ConfigManager::GetUserDataFolder() + wxFILE_SEP_PATH + filename);
-#endif
     wxString e(::DetermineExecutablePath() + wxFILE_SEP_PATH + filename);
 
     if (::wxFileExists(e))
@@ -560,6 +541,11 @@ inline wxString ConfigManager::GetUserDataFolder()
 {
     if (has_alternate_user_data_path)
         return alternate_user_data_path;
+#ifdef __WXMSW__
+    TCHAR buffer[MAX_PATH];
+    if (::GetEnvironmentVariable(_T("APPDATA"), buffer, MAX_PATH))
+        return wxString::Format(_T("%s\\CodeBlocks"), buffer);
+#endif
     return wxStandardPathsBase::Get().GetUserDataDir();
 }
 
@@ -1462,11 +1448,7 @@ void ConfigManager::Write(const wxString& name, const ConfigManagerContainer::Se
 
 void ConfigManager::InitPaths()
 {
-#ifdef __WINDOWS__
-    ConfigManager::config_folder = GetPortableConfigDir();
-#else
     ConfigManager::config_folder = ConfigManager::GetUserDataFolder();
-#endif
     ConfigManager::home_folder = wxStandardPathsBase::Get().GetUserConfigDir();
     ConfigManager::app_path = ::DetermineExecutablePath();
     wxString res_path = ::DetermineResourcesPath();
Also as attachment.

Would be nice to see this new option in trunk.

- osdt

[attachment deleted by admin]

Offline dmoore

  • Developer
  • Lives here!
  • *****
  • Posts: 1576
Re: New switch --user-config
« Reply #13 on: January 22, 2014, 04:17:00 am »
New patch. This version does a simple check for the existence of the user-data-dir, creates if not, and errors out if that doesn't work. ...
The patch doesn't work on Windows because of GetPortableConfigDir(). It is called 3 times and would always return $APPDATA/CodeBlocks on most Systems.

I forgot that there was this issue to deal with as well. Thanks for testing and the patch. However, I think we want to preserve existing behavior of allowing the user to set APPDATA on windows (even though that might have weird interactions with other apps spawned by C::B), which I think is as simple as the following one liner:

Code
Index: src/sdk/configmanager.cpp
===================================================================
--- src/sdk/configmanager.cpp   (revision 9594)
+++ src/sdk/configmanager.cpp   (working copy)
@@ -68,10 +71,10 @@
 inline wxString GetPortableConfigDir()
 {
     TCHAR buffer[MAX_PATH];
-    if (::GetEnvironmentVariable(_T("APPDATA"), buffer, MAX_PATH))
+    if (!ConfigManager::has_alternate_user_data_path && ::GetEnvironmentVariable(_T("APPDATA"), buffer, MAX_PATH))
         return wxString::Format(_T("%s\\CodeBlocks"), buffer);
     else

I also forgot to override the check for default.conf in the executable directory (I think it makes sense to obey the switch if it is specified). See the second change in this part of the patch.

Code
Index: src/sdk/configmanager.cpp
===================================================================
--- src/sdk/configmanager.cpp (revision 9594)
+++ src/sdk/configmanager.cpp (working copy)
@@ -213,11 +218,11 @@
 #ifdef __WINDOWS__
     wxString u(GetPortableConfigDir() + wxFILE_SEP_PATH + filename);
 #else
-    wxString u(wxStandardPathsBase::Get().GetUserDataDir() + wxFILE_SEP_PATH + filename);
+    wxString u(ConfigManager::GetUserDataFolder() + wxFILE_SEP_PATH + filename);
 #endif
     wxString e(::DetermineExecutablePath() + wxFILE_SEP_PATH + filename);
 
-    if (::wxFileExists(e))
+    if (!ConfigManager::has_alternate_user_data_path && ::wxFileExists(e))
     {
         ConfigManager::relo = true;
         return e;

Revised complete patch is attached
« Last Edit: January 22, 2014, 04:48:59 am by dmoore »

Offline dmoore

  • Developer
  • Lives here!
  • *****
  • Posts: 1576
Re: New switch --user-config
« Reply #14 on: January 22, 2014, 04:26:04 am »
Would be nice to see this new option in trunk.

I think this patch is pretty close. One of the other devs should weigh in re oBFusCATed's concerns and on the change in general.

One last thing: what if the user specifies a relative path?

Code
codeblocks --user-data-dir=xyz

Currently I think it will just return with error on all systems. Alternatively it could generate a directory relative to the (a) current working directory, or (b) the executable directory. Not clear either is all that useful.
« Last Edit: January 22, 2014, 04:48:37 am by dmoore »

Offline osdt

  • Multiple posting newcomer
  • *
  • Posts: 63
Re: New switch --user-config
« Reply #15 on: January 22, 2014, 06:39:47 am »
... which I think is as simple as the following one liner:
Code
Index: src/sdk/configmanager.cpp
===================================================================
--- src/sdk/configmanager.cpp   (revision 9594)
+++ src/sdk/configmanager.cpp   (working copy)
@@ -68,10 +71,10 @@
 inline wxString GetPortableConfigDir()
 {
     TCHAR buffer[MAX_PATH];
-    if (::GetEnvironmentVariable(_T("APPDATA"), buffer, MAX_PATH))
+    if (!ConfigManager::has_alternate_user_data_path && ::GetEnvironmentVariable(_T("APPDATA"), buffer, MAX_PATH))
         return wxString::Format(_T("%s\\CodeBlocks"), buffer);
     else

Yes that works also. If you want to keep GetPortableConfigDir() it has to be declared as friend of ConfigManager. But it's not needed anymore if you implement ConfigManager::GetUserDataFolder() like below:

Code
inline wxString ConfigManager::GetUserDataFolder()
{
    if (has_alternate_user_data_path)
        return alternate_user_data_path;
#ifdef __WXMSW__
    TCHAR buffer[MAX_PATH];
    if (::GetEnvironmentVariable(_T("APPDATA"), buffer, MAX_PATH))
        return wxString::Format(_T("%s\\CodeBlocks"), buffer);
#endif
    return wxStandardPathsBase::Get().GetUserDataDir();
}

I also forgot to override the check for default.conf in the executable directory (I think it makes sense to obey the switch if it is specified). See the second change in this part of the patch.
Good idea.

- osdt

Offline dmoore

  • Developer
  • Lives here!
  • *****
  • Posts: 1576
Re: New switch --user-config
« Reply #16 on: January 22, 2014, 03:20:23 pm »
osdt: Good point.

New patch merges the GetPortable_blah_blah code into GetUserDataFolder and gets rid of a bunch of #ifdef's. I haven't yet tested on windows.

Offline osdt

  • Multiple posting newcomer
  • *
  • Posts: 63
Re: New switch --user-config
« Reply #17 on: January 22, 2014, 11:56:28 pm »
Good work, the last patch works on Windows too.

One last thing: what if the user specifies a relative path?

I would vote for full path only. On Linux ~/xyz could be supported as a bonus. Maybe the other devs want to comment on this.

- osdt

Offline dmoore

  • Developer
  • Lives here!
  • *****
  • Posts: 1576
Re: New switch --user-config
« Reply #18 on: January 25, 2014, 08:56:16 pm »
Still looking for feedback on this. It works for me on both Linux and Windows. I would like to commit it next weekend (i.e. Feb 1).

Offline dmoore

  • Developer
  • Lives here!
  • *****
  • Posts: 1576
Re: New switch --user-config
« Reply #19 on: February 01, 2014, 08:26:58 pm »
Last chance... committing later today unless I hear objections.  ;D

Offline dmoore

  • Developer
  • Lives here!
  • *****
  • Posts: 1576
Re: New switch --user-config
« Reply #20 on: February 02, 2014, 01:43:38 am »
committed rev 9622