{"thread":{"id":"53727","subject":"[PATCH] haiku: add Haiku support","startedAt":"2020-06-21T18:48:13Z","lastAt":"2020-06-24T21:53:45Z","messageCount":2,"participants":["Emir Yâsin SARI","Eric Sunshine"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"400269","messageId":"35DF1103-EF57-446A-985F-051B2D3057BD@me.com","threadId":"53727","inReplyTo":null,"subject":"[PATCH] haiku: add Haiku support","fromName":"Emir Yâsin SARI","fromEmail":"bitigchi@me.com","sentAt":"2020-06-21T18:40:42Z","receivedAt":"2020-06-21T18:48:13Z","isPatch":true,"sender":{"key":"bitigchi@me.com","avatar":null},"body":"Hello,\n\nBelow you can find the current Haiku patches for upstream approval.\n\nCurrently Haiku is built with this [1] make configuration. Therefore it’s not possible to build the Haiku version with just ‘make’ yet.\nI need help in adapting this configuration to the current Git Makefile, I don’t have much experience in Makefiles.\nOr if it’s trivial, any quick-fix help is appreciated :).\n\nIn the meantime, please find the patchset below for review.\n\n[1] https://github.com/haikuports/haikuports/blob/02adc9b2c68a0ad51bc4db5699b46adc15c24c5c/dev-vcs/git/git-2.27.0.recipe#L138\n\nFrom 2ac0c135d7c12a2581fe70ed1c8ffb4809950b55 Mon Sep 17 00:00:00 2001\nFrom: Emir Sarı \nDate: Sun, 21 Jun 2020 21:02:23 +0300\nSubject: [PATCH] haiku: add Haiku support\nMIME-Version: 1.0\nContent-Type: text/plain; charset=UTF-8\nContent-Transfer-Encoding: 8bit\n\nThis commit is a collection of minor patches previously applied as a\nHaikuports patchset at github.com/haikuports/haikuports repository for\nGit Haiku port.\n\nPatchset history:\n\nFrom 56acac1a903dcbdd37c3b57fc168ad20179596b1 Mon Sep 17 00:00:00 2001\nFrom: Ingo Weinhold \nDate: Tue, 13 Aug 2013 08:07:25 +0200\nSubject: git-web--browse.sh: use \"open\" on Haiku\n\nFrom 0f557f23acaef82951851dc2165c9a9a4065703e Mon Sep 17 00:00:00 2001\nFrom: Ingo Weinhold \nDate: Mon, 19 Jan 2015 15:37:16 -0500\nSubject: On Haiku use the user settings directory instead of HOME\n\nFrom 2df42bd79662a4db16c91348283c22bddd728523 Mon Sep 17 00:00:00 2001\nFrom: Oliver Tappe \nDate: Mon, 19 Jan 2015 15:50:09 -0500\nSubject: Ensure config-directory exists before using it.\n\nFrom e2d721e9456e591cd467759a5b8fa6a9102df0b7 Mon Sep 17 00:00:00 2001\nFrom: Adrien Destugues \nDate: Sun, 14 Feb 2016 10:32:12 +0100\nSubject: Move credential cache to the config directory.\n\nFrom 5841649bb9d07c614031f95dc211e066b9f4650a Mon Sep 17 00:00:00 2001\nFrom: sfanxiang \nDate: Mon, 1 Jan 2018 13:26:28 +0000\nSubject: builtin: config: use xdg_config even if it does not exist\n\nFrom 82d3fea48bc155df46881f065349b3408c4ac75b Mon Sep 17 00:00:00 2001\nFrom: Adrien Destugues \nDate: Sun, 18 Nov 2018 11:56:26 +0100\nSubject: Fix detection of Haiku for git web browse\n\nPlus final code formatting and modifying existing patches to coexist\nwith the current code: Myself.\n\nSigned-off-by: Emir Sarı \n---\n builtin/config.c | 9 +++++++++\n config.c | 7 +++++++\n credential-cache.c | 4 ++++\n git-web--browse.sh | 5 +++++\n path.c | 14 ++++++++++++--\n 5 files changed, 37 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/config.c b/builtin/config.c\nindex ee4aef6..f9dfd08 100644\n--- a/builtin/config.c\n+++ b/builtin/config.c\n@@ -645,6 +645,14 @@ int cmd_config(int argc, const char **argv, const char *prefix)\n \t\tchar *user_config = expand_user_path(\"~/.gitconfig\", 0);\n \t\tchar *xdg_config = xdg_config_home(\"config\");\n \n+\t\t#ifdef __HAIKU__\n+\t\tif (!xdg_config) {\n+\t\t\tgiven_config_source.file = user_config;\n+\t\t} else {\n+\t\t\tgiven_config_source.file = xdg_config;\n+\t\t\tif (user_config) free(user_config);\n+\t\t}\n+\t\t#else\n \t\tif (!user_config)\n \t\t\t/*\n \t\t\t * It is unknown if HOME/.gitconfig exists, so\n@@ -664,6 +672,7 @@ int cmd_config(int argc, const char **argv, const char *prefix)\n \t\t\tgiven_config_source.file = user_config;\n \t\t\tfree(xdg_config);\n \t\t}\n+\t\t#endif\n \t}\n \telse if (use_system_config) {\n \t\tgiven_config_source.file = git_etc_gitconfig();\ndiff --git a/config.c b/config.c\nindex 8db9c77..b956ff9 100644\n--- a/config.c\n+++ b/config.c\n@@ -2745,6 +2745,7 @@ int git_config_set_multivar_in_file_gently(const char *config_filename,\n \tint ret;\n \tstruct lock_file lock = LOCK_INIT;\n \tchar *filename_buf = NULL;\n+\tchar *config_dir = NULL;\n \tchar *contents = NULL;\n \tsize_t contents_sz;\n \tstruct config_store_data store;\n@@ -2761,6 +2762,12 @@ int git_config_set_multivar_in_file_gently(const char *config_filename,\n \tif (!config_filename)\n \t\tconfig_filename = filename_buf = git_pathdup(\"config\");\n \n+\tconfig_dir = xstrdup(config_filename);\n+\t* find_last_dir_sep(config_dir) = '\\0';\n+\tif (access(config_dir, F_OK) != 0)\n+\t\tmkdir(config_dir, 0755);\n+\tfree(config_dir);\n+\n \t/*\n \t * The lock serves a purpose in addition to locking: the new\n \t * contents of .git/config will be written into it.\ndiff --git a/credential-cache.c b/credential-cache.c\nindex 1cccc3a..fe0d94c 100644\n--- a/credential-cache.c\n+++ b/credential-cache.c\n@@ -87,7 +87,11 @@ static char *get_socket_path(void)\n {\n \tstruct stat sb;\n \tchar *old_dir, *socket;\n+\t#ifdef __HAIKU__\n+\told_dir = xdg_config_home(\"credential-cache\");\n+\t#else\n \told_dir = expand_user_path(\"~/.git-credential-cache\", 0);\n+\t#endif\n \tif (old_dir && !stat(old_dir, &sb) && S_ISDIR(sb.st_mode))\n \t\tsocket = xstrfmt(\"%s/socket\", old_dir);\n \telse\ndiff --git a/git-web--browse.sh b/git-web--browse.sh\nindex ae15253..bf9135c 100755\n--- a/git-web--browse.sh\n+++ b/git-web--browse.sh\n@@ -133,6 +133,11 @@ if test -z \"$browser\" ; then\n \t\tbrowser_candidates=\"cygstart $browser_candidates\"\n \tfi\n \n+\t# BEINCLUDES indicates Haiku\n+\tif test -f $BEINCLUDES; then\n+\t\tbrowser_candidates=\"open $browser_candidates\"\n+\tfi\n+\n \tfor i in $browser_candidates; do\n \t\tinit_browser_path $i\n \t\tif type \"$browser_path\" > /dev/null 2>&1; then\ndiff --git a/path.c b/path.c\nindex 8b2c753..2fa8c56 100644\n--- a/path.c\n+++ b/path.c\n@@ -12,6 +12,10 @@\n #include \"packfile.h\"\n #include \"object-store.h\"\n #include \"lockfile.h\"\n+#ifdef __HAIKU__\n+#include \n+#include \n+#endif\n \n static int get_st_mode_bits(const char *path, int *mode)\n {\n@@ -1500,16 +1504,22 @@ int looks_like_command_line_option(const char *str)\n \n char *xdg_config_home(const char *filename)\n {\n+\t#ifdef __HAIKU__\n+\tchar settingsPath[B_PATH_NAME_LENGTH];\n+\tassert(filename);\n+\tif (find_directory(B_USER_SETTINGS_DIRECTORY, -1, true, settingsPath,\n+\t\tsizeof(settingsPath)) == B_OK)\n+\t\treturn mkpathdup(\"%s/git/%s\", settingsPath, filename);\n+\t#else\n \tconst char *home, *config_home;\n-\n \tassert(filename);\n \tconfig_home = getenv(\"XDG_CONFIG_HOME\");\n \tif (config_home && *config_home)\n \t\treturn mkpathdup(\"%s/git/%s\", config_home, filename);\n-\n \thome = getenv(\"HOME\");\n \tif (home)\n \t\treturn mkpathdup(\"%s/.config/git/%s\", home, filename);\n+\t#endif\n \treturn NULL;\n }\n \n-- \n2.27.0\n\n\n"},{"id":"400559","messageId":"CAPig+cR3Y62ff_L_ofg0wvro1idHeL9TE-E5eTsLzNqhLFdspA@mail.gmail.com","threadId":"53727","inReplyTo":"35DF1103-EF57-446A-985F-051B2D3057BD@me.com","subject":"Re: [PATCH] haiku: add Haiku support","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2020-06-24T21:53:28Z","receivedAt":"2020-06-24T21:53:45Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, Jun 21, 2020 at 2:47 PM Emir Yâsin SARI wrote:\n> Below you can find the current Haiku patches for upstream approval.\n\nThanks for volunteering to upstream Haiku-specific fixes. Please note\nthat it is unlikely that these changes will be accepted as-is, and\nwill require a fair bit of re-work to get them into shape for merging\ninto this project. If you're interested in pursuing this further, see\nmy comments below. Rather than submitting a single patch, when you\nre-roll, you will almost certainly end up submitting a multi-patch\nseries.\n\n> Currently Haiku is built with this [1] make configuration. Therefore\n> it’s not possible to build the Haiku version with just ‘make’ yet.\n> I need help in adapting this configuration to the current Git\n> Makefile, I don’t have much experience in Makefiles. Or if it’s\n> trivial, any quick-fix help is appreciated :).\n\nThese sort of 'make' tweaks are normally handled by adjusting Git's\nconfig.mak.uname file. So, you will want to add a new section to\nconfig.mak.uname which looks something like this:\n\n ifeq ($(uname_S),Haiku)\n PTHREAD_LIBS =\n USE_LIBPCRE2 = YesPlease\n NO_D_TYPE_IN_DIRENT = YesPlease\n NO_MEMMEM = YesPlease\n NEEDS_LIBICONV = YesPlease\n GNU_ROFF = YesPlease\n PERL_PATH = /bin/perl\n NO_PYTHON = YesPlease\n NO_TCLTK = YesPlease\n OBJECT_CREATION_USES_RENAMES = YesPlease\n NO_CROSS_DIRECTORY_HARDLINKS = YesPlease\n NO_INSTALL_HARDLINKS = YesPlease\n HAVE_DEV_TTY = YesPlease\n DEFAULT_EDITOR = nano\n DEFAULT_HELP_FORMAT = web\n BASIC_LDFLAGS = -lnetwork -lbsd\n endif\n\n> In the meantime, please find the patchset below for review.\n\nRight here is where your email commentary ends and the patch proper\nbegins. To allow \"git am\" to automatically pluck out the actual patch\nportion out of the email, you should insert a \"--- >8 ---\" (scissors)\nline here.\n\n> From 2ac0c135d7c12a2581fe70ed1c8ffb4809950b55 Mon Sep 17 00:00:00 2001\n> From: Emir Sarı \n> Date: Sun, 21 Jun 2020 21:02:23 +0300\n> Subject: [PATCH] haiku: add Haiku support\n> MIME-Version: 1.0\n> Content-Type: text/plain; charset=UTF-8\n> Content-Transfer-Encoding: 8bit\n\nExcept for From: and Subject:, all the other header lines are just\nnoise and of no interest to the project when submitting patches, thus\nshould not be included. (Better yet, if possible, use \"git send-email\"\nor GitGitGadget\" to send patches, as those tools will ensure the\ncorrect headers are present.)\n\n> This commit is a collection of minor patches previously applied as a\n> Haikuports patchset at github.com/haikuports/haikuports repository for\n> Git Haiku port.\n>\n> Patchset history:\n>\n> From 56acac1a903dcbdd37c3b57fc168ad20179596b1 Mon Sep 17 00:00:00 2001\n> From: Ingo Weinhold \n> Date: Tue, 13 Aug 2013 08:07:25 +0200\n> Subject: git-web--browse.sh: use \"open\" on Haiku\n\nA few comments...\n\nDrop the \"From \" header; it is not of particular\ninterest in this context.\n\nIt probably would be a good idea to indent the entire header block\nhere as a precaution against it confusing tools which automate patch\napplication or which otherwise perform some sort of automated patch\nprocessing.\n\nI'm almost tempted to suggest submitting each of these changes as\nseparate patches in order to retain attribution of each of the\nauthors. On the other hand, to get these changes accepted into this\nproject, you almost certainly will end up rewriting them so\nsignificantly that they may no longer resemble the changes from the\noriginal authors. If that's the case, it might make more sense for you\nto reference the original authors via \"Based on a patch by...\" in each\nof your new patches.\n\n> Signed-off-by: Emir Sarı \n> ---\n> diff --git a/builtin/config.c b/builtin/config.c\n> @@ -645,6 +645,14 @@ int cmd_config(int argc, const char **argv, const char *prefix)\n> char *user_config = expand_user_path(\"~/.gitconfig\", 0);\n> char *xdg_config = xdg_config_home(\"config\");\n>\n> + #ifdef __HAIKU__\n> + if (!xdg_config) {\n> + given_config_source.file = user_config;\n> + } else {\n> + given_config_source.file = xdg_config;\n> + if (user_config) free(user_config);\n> + }\n> + #else\n\nThis project frowns heavily on introducing platform-specific\nconditional compilation like this. As such, this `#ifdef __HAIKU__`\nwill prevent this change from being accepted as-is. (It's true that\nsome such code exists already, but we want to avoid adding more.)\n\nThe commit[1] from which you derived this change doesn't explain why\nthe change was made, but rather only what it changes: specifically, it\nforces use of XDG even if the XDG directory is missing rather than\nfalling back to ~/.gitconfig. This particular behavior change isn't\nsomething that need be specific to Haiku -- it might make sense on any\nplatform -- so it doesn't make sense to bundle it inside an `#ifdef\n__HAIKU__` conditional. The only way this sort of behavior change is\nlikely to be accepted into the project is if it re-worked to be\napplicable to any platform _and_ if it can be properly justified to\nconvince people that it makes sense. Moreover, it probably would\nrequire some sort of transition plan or mechanism.\n\nAdditionally, the change looks outright broken because it neglects to\ndo:\n\n given_config_source.scope = CONFIG_SCOPE_GLOBAL;\n\nlike the code in the #else arm.\n\n[1]: https://github.com/haikuports/haikuports/blob/02adc9b2c68a0ad51bc4db5699b46adc15c24c5c/dev-vcs/git/patches/git-2.27.0.patchset#L132\n\n> diff --git a/config.c b/config.c\n> @@ -2761,6 +2762,12 @@ int git_config_set_multivar_in_file_gently(const char *config_filename,\n> if (!config_filename)\n> config_filename = filename_buf = git_pathdup(\"config\");\n>\n> + config_dir = xstrdup(config_filename);\n> + * find_last_dir_sep(config_dir) = '\\0';\n> + if (access(config_dir, F_OK) != 0)\n> + mkdir(config_dir, 0755);\n> + free(config_dir);\n\nThis change comes from [2], which explains the change as:\n\n Ensure config-directory exists before using it.\n\nBut what it fails to say is under what circumstances the directory\nwouldn't exist. When would this code kick in? This is the sort of\nthing which needs to be properly explained in the commit message for a\nchange like this to be accepted. Such a change would almost certainly\nalso be accompanied by a new test, perhaps added to t/t1300-config.sh.\n\nThere are other problems with this change, as well. For instance, why\nis this \"fix\" made only to git_config_set_multivar_in_file_gently(),\nbut not to git_config_copy_or_rename_section_in_file() which almost\ncertainly suffers the same problem?\n\nMoreover, this code:\n\n * find_last_dir_sep(config_dir) = '\\0';\n\nis just outright dangerous considering that find_last_dir_sep() is\ndocumented as possibly returning NULL. (A minor additional point is\nthat it violates project style convention by having a space after the\n'*'.)\n\n[2]: https://github.com/haikuports/haikuports/blob/02adc9b2c68a0ad51bc4db5699b46adc15c24c5c/dev-vcs/git/patches/git-2.27.0.patchset#L73\n\n> diff --git a/credential-cache.c b/credential-cache.c\n> @@ -87,7 +87,11 @@ static char *get_socket_path(void)\n> {\n> + #ifdef __HAIKU__\n> + old_dir = xdg_config_home(\"credential-cache\");\n> + #else\n> old_dir = expand_user_path(\"~/.git-credential-cache\", 0);\n> + #endif\n\nThis change comes from [3] which justifies it as:\n\n Move credential cache to the config directory.\n\n Do not clutter the home dir.\n\nThe same comments from above apply...\n\n* don't insert platform-specific conditionals\n\n* there is nothing Haiku-specific about this change, thus protecting\n it inside a `#ifdef __HAIKU__` makes no sense\n\n* if this sort of behavior change is indeed desirable, then it should\n be implemented in a generic way with some sort of transition plan\n from old to new behavior\n\n[3]: https://github.com/haikuports/haikuports/blob/02adc9b2c68a0ad51bc4db5699b46adc15c24c5c/dev-vcs/git/patches/git-2.27.0.patchset#L108\n\n> diff --git a/git-web--browse.sh b/git-web--browse.sh\n> @@ -133,6 +133,11 @@ if test -z \"$browser\" ; then\n> + # BEINCLUDES indicates Haiku\n> + if test -f $BEINCLUDES; then\n> + browser_candidates=\"open $browser_candidates\"\n> + fi\n\nHaiku support was originally added to git-web--browse.sh by [4] which\ntriggered this conditional off existence of file\n/boot/system/haiku_loader, however, that ended up being unreliable, so\n[5] changed it to look for environment variable BEINCLUDES.\n\nHowever, it feels somewhat fragile to depend upon an environment\nvariable which might or might not exist in the development environment\n(even if it is present by default). A more robust alternative would be\nto check against something not likely to be overridden by the user.\nFor instance:\n\n if test \"$(uname)\" = Haiku; then\n\n(Also note that placing 'then' on the same line as 'if' goes against\nstyle in this project, however, this particular portion of\ngit-web--browse.sh already breaks style in that regard, so it's hard\nto fault it here.)\n\n[4]: https://github.com/haikuports/haikuports/blob/02adc9b2c68a0ad51bc4db5699b46adc15c24c5c/dev-vcs/git/patches/git-2.27.0.patchset#L1\n[5]: https://github.com/haikuports/haikuports/blob/02adc9b2c68a0ad51bc4db5699b46adc15c24c5c/dev-vcs/git/patches/git-2.27.0.patchset#L175\n\n> diff --git a/path.c b/path.c\n> @@ -12,6 +12,10 @@\n> +#ifdef __HAIKU__\n> +#include \n> +#include \n> +#endif\n> @@ -1500,16 +1504,22 @@ int looks_like_command_line_option(const char *str)\n> char *xdg_config_home(const char *filename)\n> {\n> + #ifdef __HAIKU__\n> + char settingsPath[B_PATH_NAME_LENGTH];\n> + assert(filename);\n> + if (find_directory(B_USER_SETTINGS_DIRECTORY, -1, true, settingsPath,\n> + sizeof(settingsPath)) == B_OK)\n> + return mkpathdup(\"%s/git/%s\", settingsPath, filename);\n> + #else\n\nAs noted earlier, don't introduce platform-specific compilation\nsections like this. The way this is normally handled is to place\nplatform-specific implementation in a file in the compat/ directory\n(perhaps \"compat/haiku.c\", for instance), and then add a #define to\ngit-compat-util.h to cause the platform-specific implementation to be\ninvoked in place of the generic implementation. See compat/mingw.[ch]\nand git-compat-util.h for examples of how this is done.\n\nTo actually make that work, you might need to do a bit of preparation\nto make it possible to swap in a different implementation of\nxdg_config_home(), though.\n\nA bit of commentary on the code itself: In this project, the variable\nwould be named \"settings_path\", not \"settingsPath\". That's something\nto keep in mind when writing code for generic parts of the system. If\nyou're placing this code in a platform-specific compat/haiku.c file,\non the other hand, you may be able to get away with breaking style and\ngoing with \"settingsPath\" (though if there is no strong need to break\nstyle, then why bother?).\n\n> const char *home, *config_home;\n> -\n> assert(filename);\n> config_home = getenv(\"XDG_CONFIG_HOME\");\n> if (config_home && *config_home)\n> return mkpathdup(\"%s/git/%s\", config_home, filename);\n> -\n> home = getenv(\"HOME\");\n\nDon't make arbitrary changes like this unrelated to the purpose of a\npatch. Doing so gives reviewers extra work with no benefit.\n"}]}