{"thread":{"id":"56797","subject":"[Discussion] The architecture of Scalar (and others) within Git","startedAt":"2021-10-27T21:51:11Z","lastAt":"2022-06-24T21:53:48Z","messageCount":16,"participants":["Derrick Stolee","Johannes Schindelin","Philip Oakley","Ævar Arnfjörð Bjarmason","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"439839","messageId":"b67bbef4-e4c3-b6a7-1c7f-7d405902ef8b@gmail.com","threadId":"56797","inReplyTo":null,"subject":"[Discussion] The architecture of Scalar (and others) within Git","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2021-10-27T21:51:05Z","receivedAt":"2021-10-27T21:51:11Z","isPatch":false,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"The discussion happening on \"Upstreaming the Scalar command\" [1] has\ngotten pretty deep into some details about builds and code organization.\nWhile these are important things to discuss, I’d like to step back and\nreframe the discussion to focus on how components like Scalar fit into the\nhigh-level architecture of Git. The decisions we make for Scalar also\naffect other components that might show up in the future. It could also\nclarify some of the components already in Git, both in and out of the\n`contrib/` directory.\n\n[1] https://lore.kernel.org/git/pull.1005.git.1630359290.gitgitgadget@gmail.com/\n\nI'm starting this discussion thread to create a new area to consider these\nhigh-level concepts. Specifically: how should a new component like Scalar\nbe included in the Git codebase?\n\nJohannes put details in his cover letter about why we put it in\n`contrib/scalar/`, but I feel like code review proceeded at first as if\nthat was a foregone conclusion. Then, discussion shifted to some\nalternatives, but the reasons were focused on code size and specific\ntweaks to the `Makefile`. I think there are other concerns for the entire\nGit community that need to be included in the calculus. In particular,\nthe decisions we make today will become precedents for similar\ncontributions in the future.\n\nI'll start by setting up some goals and guidelines for the decision\nprocess, then get into some concrete options before comparing them based\non these guidelines. I also try to separate the \"final\" goal from \"how do\nwe get there\" in the last section.\n\nAll of what I say is up for debate. I would love for others to contribute\noptions that I missed. I will send a reply to this message that summarizes\nall options that are recommended in this thread, along with some attempt\nto summarize any consensus that we might be approaching.\n\n\nGoals\n-----\n\nOur main goal is to identify a community-supported plan for upstreaming the\nScalar CLI.\n\nThis comes in two parts:\n\n1. What is the preferred final organization of the Scalar code within\n the Git codebase?\n\n2. What is a realistic approach to contributing the complete Scalar CLI\n with incrementally-reviewable pieces, in order to end up with the\n preferred final organization?\n\n\nOptimization Factors\n--------------------\n\nThere are multiple factors involved that we wish to optimize. Here is my\nunderstanding of the most-important factors and how they interact:\n\n### Value\n\nThe main reason to include Scalar within the Git codebase is to make it\navailable to more users. This should have downstream effects based on user\nfeedback about what they like or dislike about the Scalar CLI that can\ninform future features in core Git that simplify repository management at\nscale.\n\n### CLI compatibility\n\nThe Scalar CLI was developed externally, which informed the choice of\ncommands and arguments. We prefer to maintain the existing CLI design to\nsupport our existing customers without disruption.\n\nAfter the existing CLI is completely contributed, we can consider making\nchanges to the CLI by deprecating certain options and replacing them with\nsynonyms. (For example, my personal regret is that `--full-clone` really\nmeans `--no-sparse`.)\n\n### Developer friction\n\nOnce the Scalar CLI is in the Git codebase, it becomes something that can\ndisrupt Git developers as they modify Git code, especially within libgit.a.\nWhat is the right balance of making developers responsible for breaking\nthe Scalar CLI as they make changes to Git?\n\n### Git Maintainer burden\n\nSimilar to developer friction, how much does having Scalar in the Git\ncodebase affect the Git maintainer? Does the Git maintainer become\nresponsible for the maintenance of Scalar? Do contributions to Scalar need\nthe same scrutiny as other contributions? Does the maintainer need to\ninform contributors when they break Scalar?\n\n### Scalar Maintainer(s) responsibility\n\nAt the very last, what are the roles of \"the Scalar maintainer(s)\"? Here,\nI'm thinking of Johannes and myself as the accountable parties. An example\nof a responsibility here could be that around release time we validate\nthat Scalar is still working with the changes that happened during that\nrelease cycle.\n\nThe biggest reason to include the Scalar maintainers' responsibility is\nthat the options listed below present a converse relationship between\nthese responsibilities and those of contributors and the Git maintainer.\nThe more responsibility that the Git community is willing to accept, the\nless responsibility on the Scalar maintainers. We are _not_ optimizing to\nminimize our responsibility, but some options change the balance of\nresponsibility.\n\n\nOptions for preferred end state\n-------------------------------\n\nLet's get into some concrete proposals for the location of the Scalar CLI\nwithin the Git codebase. These are ordered based on increasing\nresponsibility on the Git community: the first option should minimize\ncommunity responsibility and maximize responsibility on the Scalar\nmaintainers. We will discuss the pros and cons of each option after fully\ndescribing each of them.\n\n(Please add any options that I may have missed!)\n\n**Option 1:** One-directional coupling in `contrib/scalar/`\n\nThe Scalar CLI exists entirely within `contrib/scalar/`, including all code\nand test infrastructure. To opt-in to building and testing Scalar, run\n`make -C contrib/scalar test` to build Git and Scalar and run the Scalar\ntest scripts.\n\n**Option 2:** Loose coupling in a new scalar/ directory\n\nSimilar to `git-gui` and `gitk-git`, Scalar could exist outside of `contrib/`\nand in its own `scalar/` directory. Its code is isolated to this directory,\nbut its tests live in `t/` and its documentation lives in `Documentation/`.\nThe root `Makefile` has special logic to avoid building and testing Scalar\nunless an `INCLUDE_SCALAR` option is enabled. The `DEVELOPER=1` option\nenables `INCLUDE_SCALAR` by default. Code in `git.c` is generalized so\n`scalar.c` could take advantage of features such as `-c`, `-C`, and\n`--exec-path` options.\n\n**Option 3:** Tight coupling with entire Git project\n\nScalar is included at the root as `scalar.c` and is compiled at the same\npriority as the `git` executable. Tests and documentation have the same\npriority as other Git features. Code in `git.c` is generalized so\n`scalar.c` could take advantage of features such as `-c`, `-C`, and\n`--exec-path` options.\n\nNote: there are likely more subtle gradients between these options, but\nOption 1 and Option 3 are intended as extreme ends of a spectrum. Option\n2 is a potential middle ground, but a lot of the details about it could be\naltered.\n\n\nBenefits and drawbacks of these options\n---------------------------------------\n\nHere are some pros and cons for each approach. Please reply with additional\nitems to add to the list, or to debate the correctness of each item. I'll\ntry to summarize these contributions in a reply to this message after the\ndiscussion stabilizes.\n\n**Option 1:** One-directional coupling in `contrib/scalar/`\n\nThis option was our initial choice because it minimizes the responsibility\nof the Git community. While `contrib/scalar/scalar.c` depends on code in\n`libgit.a`, the implementation does not require that code to change to\naccommodate the needs of Scalar. The test suite and documentation is\nseparate.\n\nThis does mean that changes to `libgit.a` could break Scalar without any\nfeedback to the developer that has not compiled Scalar. The Scalar\nmaintainers would then need to watch for this and send separate updates to\nScalar that fix these dependency breaks. This reduces developer friction,\nbut might cause some extra burden on the Git maintainer. If these \"catch\nup to dependency breaks\" updates happen only after a release candidate is\nout, then maybe this isn't too much of a burden. We don't typically have\nrelease candidates for minor releases, so there is some risk there that\nminor releases could include breaks.\n\nIf we make our CI builds include Scalar by default, then the previous\nparagraph about developer friction is negated.\n\nHaving Scalar in `contrib/` makes it easy to differentiate it as an\noptional component that distributors could choose to include or leave out.\n\nCode in `contrib/` has a lower barrier to entry. In particular, the Scalar\nCLI is not intended to be up for debate for historical reasons. If we make\nan exception for Scalar but want Scalar integrated outside of `contrib/`,\nthen are we setting expectations for other tools that might want to be\nincluded?\n\nHowever, projects within `contrib/` do not currently depend on `libgit.a`\nthe way Scalar does. (This is not historically true, as an older project\nad such dependencies, but has since been ejected.) This leads to questions\nof whether or not this is a desirable pattern to follow. It adds to\ncomplexity since changes to the core Git codebase can break something in\n`contrib/` at compile time.\n\nBy keeping the code dependency one-directional, some nice-to-have features\ncan be duplicated within `contrib/scalar/` to avoid disrupting their\nimplementation within the core Git codebase.\n\nThis option explicitly mentions that all knowledge of how to build Scalar\nlives within `contrib/scalar/` to avoid disrupting the core `Makefile`.\nThis has already led to debate about some duplication in the Scalar\n`Makefile` and the one at root. Some functionality might not be critical\nto include in both places. One knob that we can use to adjust this option\nis to be more flexible with the root `Makefile` knowing about code within\n`contrib/scalar`. This violates existing patterns except for how the\n`make coccicheck` target consumes scripts from `contrib/coccinelle` to do\nchecks on the Git source code.\n\n**Option 2:** Loose coupling in a `scalar/` directory\n\nThe first issue with this option is that the Scalar CLI is established and\nis not up for modification, yet we would be contributing it in a location\nthat is typically under high scrutiny for things like this.\n\nThis option also breaks into new territory, because even `git-gui` and\n`gitk-git` are not based on C and do not depend on `libgit.a`.\n\nThere is some risk that as we refactor some things in the Git build system\nand codebase to allow Scalar to plug in more tightly that we accidentally\nbreak something.\n\nDeveloper friction increases as changes to `libgit.a` that break Scalar\nshould be fixed within the patches that cause those changes. This means\nthat changes to the Scalar project can happen in otherwise-unrelated\ntopics, increasing the attention of both Git and Scalar maintainers during\nthe full release cycle. However, it does reduce risk that Scalar could be\nbroken for a significant chunk of time.\n\nThis approach reduces code duplication.\n\nNot only does the code need to be compiled optionally, we need to make\nsure that tests only run if Scalar was compiled. We also want to make sure\nthat documentation is compiled with the same options. This could affect\nthings like git-scm.com which hosts the Git documentation: should it\ninclude the Scalar docs if it isn't shipped by default? How can we make\nsure that these docs don't accidentally appear there? (Or, should we\nmake sure the docs clearly state that `scalar` might not be available?)\n\n**Option 3:** Tight coupling with the entire Git project\n\nThis option removes the ability to opt-out of building the Scalar CLI.\nDistributors might need to react to remove the `scalar` executable if they\ndo not want to include it.\n\nThis option relies on significant buy-in from the Git community.\n\nThis option minimizes code duplication and has the simplest build system.\n\nThis option sets a risky precedent that new tools can be added to Git\nwithout a rigorous review of the CLI.\n\n\nPossible approaches to land in our final target\n-----------------------------------------------\n\nIf we agree on option 1 (`contrib/scalar/`), then this discussion is moot,\nbecause there is no strategy other than to contribute into `contrib/` from\nthe start.\n\nIf we agree on another option, then we could still continue contributing\nScalar into `contrib/`, following the currently-proposed organization. There\nmight be benefits to a gradual approach to reaching that target. Here is a\npossible multi-stage approach to land in our final approach:\n\n**Phase 1.** Keep code in contrib/ while we contribute the Scalar features.\n\nSince the current patches on the mailing list are not feature complete, it\ncan be beneficial to move forward with the code in `contrib/scalar/` until\nwe reach feature parity. At that point, we could move the source into its\nfinal resting place.\n\nThis leans into the fact that `contrib/` contains \"interesting tools...\nmaybe even experimental ones\". While Scalar is not feature complete, it\ncan be isolated as experimental here until it is ready for promotion to\nnon-experimental.\n\nDepending on the final goal, we could drop some of the work that is currently\nbuilt within the `contrib/scalar/` directory, such as `-c`/`-C` parsing or\ndocumentation builds. These features would be reimplemented in the new\nlocation, so we can prevent that duplicate effort if we have a different\nfinal location in mind.\n\n**Phase 2.** Establish community standards on optional components\n\nWhile the work in `contrib/scalar/` continues to reach feature parity, we\ncan work as a community to set some ground rules about these kinds of\noptional features (depending on how \"optional\" we land). This can include\nstandards such as how the files are laid out in the repository and how\nthey interact with the `Makefile`. Whatever we do for Scalar is likely to\nform an example for a future contribution that we can't predict today.\nMany of the questions we are asking today can be written for posterity:\n\n* What is an appropriate level of coupling with the Git codebase?\n* Where should code custom to the component live?\n* How should documentation and tests be organized?\n* How do we initialize builds of a component?\n* Who is responsible for supporting the component? Who fixes the bugs?\n* Who is responsible for reviewing changes to the component?\n\n**Phase 3.** Translate the Scalar code from `contrib/scalar/` to new home\n\nAfter feature parity is reached _and_ we have established some ground rules\nfor how these kinds of components should fit within the Git codebase, we\ncan start to translate Scalar into its new home. This should include some\nfile renames along with some modifications to fit into the build. The goal\nof this phase is to establish the new build environment and how Scalar fits\nwithin the CI builds. Once this is complete, whatever responsibility was\nagreed upon by the community to keep Scalar working goes into effect.\n\n**Phase 4.** Integrate features that were delayed until final home\n\nAfter the code, docs, and tests have been moved, we can start the work of\nadding the features that might have been delayed until the final home. The\nmain example on my mind is the `-c`/`-C`/`--exec-path` parsing, which\nshould be its own series. Features that fit into this category include\nanything that requires modifying existing code outside of `contrib/` to be\nconsumed by Scalar.\n\n\nCall to action\n--------------\n\nThanks for your attention to this topic. I look forward to any new \"big\"\nideas in this space.\n\nThanks,\n-Stolee\n\n\n"},{"id":"439858","messageId":"nycvar.QRO.7.76.6.2110280852200.56@tvgsbejvaqbjf.bet","threadId":"56797","inReplyTo":"b67bbef4-e4c3-b6a7-1c7f-7d405902ef8b@gmail.com","subject":"Re: [Discussion] The architecture of Scalar (and others) within Git","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-10-28T06:56:00Z","receivedAt":"2021-10-28T06:56:22Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Stolee,\n\nOn Wed, 27 Oct 2021, Derrick Stolee wrote:\n\n> [...]\n> Options for preferred end state\n> -------------------------------\n>\n> Let's get into some concrete proposals for the location of the Scalar CLI\n> within the Git codebase. These are ordered based on increasing\n> responsibility on the Git community: the first option should minimize\n> community responsibility and maximize responsibility on the Scalar\n> maintainers. We will discuss the pros and cons of each option after fully\n> describing each of them.\n>\n> [...]\n\nThank you so much for summarizing all of that!\n\nI have voiced my opinion before, so I won't bore you with a rerun of that\nshow. Now, I am eager to sit back and listen to other informed opinions,\nopinions I have not heard yet. In particular Junio's... :-)\n\nCiao,\nDscho\n"},{"id":"439876","messageId":"ce6a1112-e960-cf1d-5e3f-60ef0de7abea@iee.email","threadId":"56797","inReplyTo":"b67bbef4-e4c3-b6a7-1c7f-7d405902ef8b@gmail.com","subject":"Re: [Discussion] The architecture of Scalar (and others) within Git","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.email","sentAt":"2021-10-28T15:29:36Z","receivedAt":"2021-10-28T15:29:42Z","isPatch":false,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"Hi Stollee,\na few very generic points with my \"systems\" hat on.\n\nOn 27/10/2021 22:51, Derrick Stolee wrote:\n> The discussion happening on \"Upstreaming the Scalar command\" [1] has\n> gotten pretty deep into some details about builds and code organization.\n> While these are important things to discuss, I’d like to step back and\n> reframe the discussion to focus on how components like Scalar fit into the\n> high-level architecture of Git. The decisions we make for Scalar also\n> affect other components that might show up in the future. It could also\n> clarify some of the components already in Git, both in and out of the\n> `contrib/` directory.\n>\n> [1] https://lore.kernel.org/git/pull.1005.git.1630359290.gitgitgadget@gmail.com/\n>\n> I'm starting this discussion thread to create a new area to consider these\n> high-level concepts. Specifically: how should a new component like Scalar\n> be included in the Git codebase?\n>\n> Johannes put details in his cover letter about why we put it in\n> `contrib/scalar/`, but I feel like code review proceeded at first as if\n> that was a foregone conclusion. Then, discussion shifted to some\n> alternatives, but the reasons were focused on code size and specific\n> tweaks to the `Makefile`. I think there are other concerns for the entire\n> Git community that need to be included in the calculus. In particular,\n> the decisions we make today will become precedents for similar\n> contributions in the future.\nWe probably should also summarise the status of and relationships with\nthe existing 'integrations' like `git-gui`, `gitk`, `Git for-Windows`, etc.\n\n>\n> I'll start by setting up some goals and guidelines for the decision\n> process, then get into some concrete options before comparing them based\n> on these guidelines. I also try to separate the \"final\" goal from \"how do\n> we get there\" in the last section.\n>\n> All of what I say is up for debate. I would love for others to contribute\n> options that I missed. I will send a reply to this message that summarizes\n> all options that are recommended in this thread, along with some attempt\n> to summarize any consensus that we might be approaching.\n>\n>\n> Goals\n> -----\n>\n> Our\nI'm assuming that 'our' here is the corporate team that has been\nendeavouring to use Git effectively on it's mega-repo and give back what\nit has learnt to Git.\n\n> main goal is to identify a community-supported plan for upstreaming the\n> Scalar CLI.\n\nHere, 'scalar' is about large code mono-repos, rather than any operating\nsystem/vendor issues, or visualisation tools. (may need saying)\n>\n> This comes in two parts:\n>\n> 1. What is the preferred final organization of the Scalar code within\n> the Git codebase?\n>\n> 2. What is a realistic approach to contributing the complete Scalar CLI\n> with incrementally-reviewable pieces, in order to end up with the\n> preferred final organization?\n>\n>\n> Optimization Factors\n> --------------------\n>\n> There are multiple factors involved that we wish to optimize. Here is my\n> understanding of the most-important factors and how they interact:\n>\n> ### Value\n>\n> The main reason to include Scalar within the Git codebase is to make it\n> available to more users. This should have downstream effects based on user\n> feedback about what they like or dislike about the Scalar CLI that can\n> inform future features in core Git that simplify repository management at\n> scale.\n>\n> ### CLI compatibility\n>\n> The Scalar CLI was developed externally, which informed the choice of\n> commands and arguments. We prefer to maintain the existing CLI design to\n> support our existing customers without disruption.\n>\n> After the existing CLI is completely contributed, we can consider making\n> changes to the CLI by deprecating certain options and replacing them with\n> synonyms. (For example, my personal regret is that `--full-clone` really\n> means `--no-sparse`.)\n>\n> ### Developer friction\n>\n> Once the Scalar CLI is in the Git codebase, it becomes something that can\n> disrupt Git developers as they modify Git code, especially within libgit.a.\n> What is the right balance of making developers responsible for breaking\n> the Scalar CLI as they make changes to Git?\n>\n> ### Git Maintainer burden\n>\n> Similar to developer friction, how much does having Scalar in the Git\n> codebase affect the Git maintainer? Does the Git maintainer become\n> responsible for the maintenance of Scalar? Do contributions to Scalar need\n> the same scrutiny as other contributions? Does the maintainer need to\n> inform contributors when they break Scalar?\nHow to quantify the size of the scalar _code_ contribution, and how is\nconcept ownership handed over?\n>\n> ### Scalar Maintainer(s) responsibility\n>\n> At the very last, what are the roles of \"the Scalar maintainer(s)\"? Here,\n> I'm thinking of Johannes and myself as the accountable parties. An example\n> of a responsibility here could be that around release time we validate\n> that Scalar is still working with the changes that happened during that\n> release cycle.\n\nEnsuring a separation of concerns (partitioning) will be important if\nthe only mega repo that can really 'battle test' the capability is to be\nseparate from the ownership of the coded concepts.\n>\n> The biggest reason to include the Scalar maintainers' responsibility is\n> that the options listed below present a converse relationship between\n> these responsibilities and those of contributors and the Git maintainer.\n> The more responsibility that the Git community is willing to accept, the\n> less responsibility on the Scalar maintainers. We are _not_ optimizing to\n> minimize our responsibility, but some options change the balance of\n> responsibility.\n\nI'm presuming this (responsibility) is with respect to the *transition*\nfrom the independent `scalar` to the Git integrated _scalar_ capability.\n>\n>\n> Options for preferred end state\n> -------------------------------\n>\n> Let's get into some concrete proposals for the location of the Scalar CLI\n> within the Git codebase. These are ordered based on increasing\n> responsibility on the Git community: the first option should minimize\n> community responsibility and maximize responsibility on the Scalar\n> maintainers. We will discuss the pros and cons of each option after fully\n> describing each of them.\n>\n> (Please add any options that I may have missed!)\n>\n> **Option 1:** One-directional coupling in `contrib/scalar/`\n>\n> The Scalar CLI exists entirely within `contrib/scalar/`, including all code\n> and test infrastructure. To opt-in to building and testing Scalar, run\n> `make -C contrib/scalar test` to build Git and Scalar and run the Scalar\n> test scripts.\n>\n> **Option 2:** Loose coupling in a new scalar/ directory\n>\n> Similar to `git-gui` and `gitk-git`, Scalar could exist outside of `contrib/`\n> and in its own `scalar/` directory. Its code is isolated to this directory,\n> but its tests live in `t/` and its documentation lives in `Documentation/`.\n> The root `Makefile` has special logic to avoid building and testing Scalar\n> unless an `INCLUDE_SCALAR` option is enabled. The `DEVELOPER=1` option\n> enables `INCLUDE_SCALAR` by default. Code in `git.c` is generalized so\n> `scalar.c` could take advantage of features such as `-c`, `-C`, and\n> `--exec-path` options.\n>\n> **Option 3:** Tight coupling with entire Git project\n>\n> Scalar is included at the root as `scalar.c` and is compiled at the same\n> priority as the `git` executable. Tests and documentation have the same\n> priority as other Git features. Code in `git.c` is generalized so\n> `scalar.c` could take advantage of features such as `-c`, `-C`, and\n> `--exec-path` options.\n>\n> Note: there are likely more subtle gradients between these options, but\n> Option 1 and Option 3 are intended as extreme ends of a spectrum. Option\n> 2 is a potential middle ground, but a lot of the details about it could be\n> altered.\nThis appears to list only end points, rather than transitional\napproaches (cf. cut & cover, tunnel, bridging) to providing some or all\nof the scalar mono-repo capability to Git.\n>\n> Benefits and drawbacks of these options\n> ---------------------------------------\n>\n> Here are some pros and cons for each approach. Please reply with additional\n> items to add to the list, or to debate the correctness of each item. I'll\n> try to summarize these contributions in a reply to this message after the\n> discussion stabilizes.\n>\n> **Option 1:** One-directional coupling in `contrib/scalar/`\n>\n> This option was our initial choice because it minimizes the responsibility\n> of the Git community. While `contrib/scalar/scalar.c` depends on code in\n> `libgit.a`, the implementation does not require that code to change to\n> accommodate the needs of Scalar. The test suite and documentation is\n> separate.\n>\n> This does mean that changes to `libgit.a` could break Scalar without any\n> feedback to the developer that has not compiled Scalar. The Scalar\n> maintainers would then need to watch for this and send separate updates to\n> Scalar that fix these dependency breaks. This reduces developer friction,\n> but might cause some extra burden on the Git maintainer. If these \"catch\n> up to dependency breaks\" updates happen only after a release candidate is\n> out, then maybe this isn't too much of a burden. We don't typically have\n> release candidates for minor releases, so there is some risk there that\n> minor releases could include breaks.\n>\n> If we make our CI builds include Scalar by default, then the previous\n> paragraph about developer friction is negated.\n>\n> Having Scalar in `contrib/` makes it easy to differentiate it as an\n> optional component that distributors could choose to include or leave out.\n>\n> Code in `contrib/` has a lower barrier to entry. In particular, the Scalar\n> CLI is not intended to be up for debate for historical reasons.\n\nAre we talking ('not for debate') about the existing `scalar`\n(corporate) cli or the potential for a medium term transition to a git\ncli embedding the _scalar_ concepts. \n> If we make\n> an exception for Scalar but want Scalar integrated outside of `contrib/`,\n> then are we setting expectations for other tools that might want to be\n> included?\n>\n> However, projects within `contrib/` do not currently depend on `libgit.a`\n> the way Scalar does. (This is not historically true, as an older project\n> ad such dependencies, but has since been ejected.) This leads to questions\n> of whether or not this is a desirable pattern to follow. It adds to\n> complexity since changes to the core Git codebase can break something in\n> `contrib/` at compile time.\n>\n> By keeping the code dependency one-directional, some nice-to-have features\n> can be duplicated within `contrib/scalar/` to avoid disrupting their\n> implementation within the core Git codebase.\n>\n> This option explicitly mentions that all knowledge of how to build Scalar\n> lives within `contrib/scalar/` to avoid disrupting the core `Makefile`.\n> This has already led to debate about some duplication in the Scalar\n> `Makefile` and the one at root. Some functionality might not be critical\n> to include in both places. One knob that we can use to adjust this option\n> is to be more flexible with the root `Makefile` knowing about code within\n> `contrib/scalar`. This violates existing patterns except for how the\n> `make coccicheck` target consumes scripts from `contrib/coccinelle` to do\n> checks on the Git source code.\n>\n> **Option 2:** Loose coupling in a `scalar/` directory\n>\n> The first issue with this option is that the Scalar CLI is established and\n> is not up for modification, yet we would be contributing it in a location\n> that is typically under high scrutiny for things like this.\n>\n> This option also breaks into new territory, because even `git-gui` and\n> `gitk-git` are not based on C and do not depend on `libgit.a`.\n>\n> There is some risk that as we refactor some things in the Git build system\n> and codebase to allow Scalar to plug in more tightly that we accidentally\n> break something.\n>\n> Developer friction increases as changes to `libgit.a` that break Scalar\n> should be fixed within the patches that cause those changes. This means\n> that changes to the Scalar project can happen in otherwise-unrelated\n> topics, increasing the attention of both Git and Scalar maintainers during\n> the full release cycle. However, it does reduce risk that Scalar could be\n> broken for a significant chunk of time.\n>\n> This approach reduces code duplication.\n>\n> Not only does the code need to be compiled optionally, we need to make\n> sure that tests only run if Scalar was compiled. We also want to make sure\n> that documentation is compiled with the same options. This could affect\n> things like git-scm.com which hosts the Git documentation: should it\n> include the Scalar docs if it isn't shipped by default? How can we make\n> sure that these docs don't accidentally appear there? (Or, should we\n> make sure the docs clearly state that `scalar` might not be available?)\n>\n> **Option 3:** Tight coupling with the entire Git project\n>\n> This option removes the ability to opt-out of building the Scalar CLI.\n> Distributors might need to react to remove the `scalar` executable if they\n> do not want to include it.\n>\n> This option relies on significant buy-in from the Git community.\n>\n> This option minimizes code duplication and has the simplest build system.\n>\n> This option sets a risky precedent that new tools can be added to Git\n> without a rigorous review of the CLI.\n>\n>\n> Possible approaches to land in our final target\n> -----------------------------------------------\n>\n> If we agree on option 1 (`contrib/scalar/`), then this discussion is moot,\n> because there is no strategy other than to contribute into `contrib/` from\n> the start.\n>\n> If we agree on another option, then we could still continue contributing\n> Scalar into `contrib/`, following the currently-proposed organization. There\n> might be benefits to a gradual approach to reaching that target. Here is a\n> possible multi-stage approach to land in our final approach:\n\nI'm minded toward seeing a transitional approach (aka: staged, gated,\n..) that would bring capability into Git, but I'm not sure about the\nlibgit.a linkage.\n>\n> **Phase 1.** Keep code in contrib/ while we contribute the Scalar features.\n>\n> Since the current patches on the mailing list are not feature complete, it\n> can be beneficial to move forward with the code in `contrib/scalar/` until\n> we reach feature parity. At that point, we could move the source into its\n> final resting place.\n>\n> This leans into the fact that `contrib/` contains \"interesting tools...\n> maybe even experimental ones\". While Scalar is not feature complete, it\n> can be isolated as experimental here until it is ready for promotion to\n> non-experimental.\n>\n> Depending on the final goal, we could drop some of the work that is currently\n> built within the `contrib/scalar/` directory, such as `-c`/`-C` parsing or\n> documentation builds. These features would be reimplemented in the new\n> location, so we can prevent that duplicate effort if we have a different\n> final location in mind.\n>\n> **Phase 2.** Establish community standards on optional components\n>\n> While the work in `contrib/scalar/` continues to reach feature parity, we\n> can work as a community to set some ground rules about these kinds of\n> optional features (depending on how \"optional\" we land). This can include\n> standards such as how the files are laid out in the repository and how\n> they interact with the `Makefile`. Whatever we do for Scalar is likely to\n> form an example for a future contribution that we can't predict today.\n> Many of the questions we are asking today can be written for posterity:\n>\n> * What is an appropriate level of coupling with the Git codebase?\n> * Where should code custom to the component live?\n> * How should documentation and tests be organized?\n> * How do we initialize builds of a component?\n> * Who is responsible for supporting the component? Who fixes the bugs?\n> * Who is responsible for reviewing changes to the component?\n>\n> **Phase 3.** Translate the Scalar code from `contrib/scalar/` to new home\n>\n> After feature parity is reached _and_ we have established some ground rules\n> for how these kinds of components should fit within the Git codebase, we\n> can start to translate Scalar into its new home. This should include some\n> file renames along with some modifications to fit into the build. The goal\n> of this phase is to establish the new build environment and how Scalar fits\n> within the CI builds. Once this is complete, whatever responsibility was\n> agreed upon by the community to keep Scalar working goes into effect.\n>\n> **Phase 4.** Integrate features that were delayed until final home\n>\n> After the code, docs, and tests have been moved, we can start the work of\n> adding the features that might have been delayed until the final home. The\n> main example on my mind is the `-c`/`-C`/`--exec-path` parsing, which\n> should be its own series. Features that fit into this category include\n> anything that requires modifying existing code outside of `contrib/` to be\n> consumed by Scalar.\n>\n>\n> Call to action\n> --------------\n>\n> Thanks for your attention to this topic. I look forward to any new \"big\"\n> ideas in this space.\n>\n> Thanks,\n> -Stolee\n>\n>\nregards\nPhilip\n"},{"id":"439920","messageId":"patch-1.1-86fb8d56307-20211028T185016Z-avarab@gmail.com","threadId":"56797","inReplyTo":"b67bbef4-e4c3-b6a7-1c7f-7d405902ef8b@gmail.com","subject":"[PATCH] contrib: build & test scalar by default, optionally install","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-28T18:56:06Z","receivedAt":"2021-10-28T18:57:33Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change how the newly proposed \"scalar\" command is handle, this builds\non top of the proposed patches that add it along with documentation\nand tests to the \"contrib/scalar/\" directory[1].\n\nWith those patches we unconditionally build scalar.o and link it to\nlibgit.a, but don't build the \"scalar\" binary itself, and don't run\nits tests as part of our normal test suite run or CI.\n\nAs the changes to \"t/t9099-scalar.sh\" here shows that sort of\narrangement leads to breakages. I.e. the scalar tests would fail on\nOSX, which our OSX CI jobs will catch, but only if we run the\ntests. Let's add a prerequisite to test that, which requires moving\naround some of the setup code.\n\nI think that for a libgit.a-using \"scalar\" in particular, and for\n\"contrib\" in general we're better off by not only compiling the object\nin question, but also linking it, and running its tests by\ndefault. What we won't do is install it by default, unless an\n\"INSTALL_SCALAR=Y\" is provided along with \"make install\".\n\nFor context: There's been ongoing discussion about how this command\nshould be integrated in terms of the \"closeness\" of its integration,\nand how \"contrib\" sources within git.git should be organized.\n\nThat discussion started with my [2], was followed by a WIP patch[3] to\nimplement the approach being finished up here, and has spawned the\n\"[Discussion] The architecture of Scalar (and others) within Git\"\nthread[4]. See also the large earlier discussion hanging off [3].\n\nI really don't think it's important whether a given source file lives\nin-tree at \"contrib/scalar/scalar.c\" or the top-level \"scalar.c\", but\nas the diff below shows the latter approach is a smaller overall\nchange in our build system, due to how it's organized, more details on\nthat below under \"Build changes\".\n\nI think that part of the reason for sticking the new command in\n\"contrib/scalar/\" was to implicitly make it clear from the paths that\nit was different. I do think that would be a worthwhile goal in the\nabstract.\n\nBut given the build simplifications we can attain by moving it to the\ntop level that we should seek to resolve that ambiguity in the minds\nof any potential git.git code maintainers in some other way.\n\nTo do that we now have a new 'OPTIONAL CONTRIB COMMANDS' section in\n\"git help git\" explaining our backwards and forwards compatibility\nnon-promises when it comes to this and similar future commands. Our\nusers are the ones most likely to be confused about why their git\npackage has installed a \"/usr/bin/scalar\" that seemingly duplicates\nparts of git's own functionality. We're much better off by\ncross-linking our documentation, and mentioning the state of scalar in\ngit's own documentation, along with the full list of porcelain and\nplumbing utilities.\n\n= Build changes =\n\nThis fixes dependency bugs in the previous \"contrib/scalar/Makefile\", as\nwell as implementing various missing bits of functionality, most\nnotably \"make install\" will now install the \"scalar\" command, but only\nif INSTALL_SCALAR is defined.\n\nThose and other changes and non-changes, categorized as \"Same\", \"New\"\nand \"Fixed\" below:\n\n== Same ==\n\nA.: We'll unconditionally build scalar.o, as before it's in the\n $(OBJECTS) list.\n\nB.: \"make contrib/scalar/scalar.o\" has the same dependency graph as\n before, but is now spelled \"make scalar.o\", more on the rename below.\n\n== New ==\n\nC.: We'll now unconditionally build and test the scalar command.\nBefore we'd only build scalar.o, but not link it.\n\n Its test lives in \"t/t9099-scalar.sh\" now (and take <1s to\n run). We should have test coverage of this in-tree command that's\n linking to libgit.a.\n\n Previously it had to be manually tested by cd-ing to\n contrib/scalar and running \"make test\", and it would not benefit\n from the combination of our various CI targets.\n\nD.: We'll unconditionally build scalar's documentation, and it will be\n linted along with the rest, including checking for broken links to\n other documentation.\n\n The minor change to scalar.txt here is needed due to the lint\n check added in cafd9828e89 (doc lint: lint and fix missing \"GIT\"\n end sections, 2021-04-09), perhaps we should have a different end\n blurb for this command, but for now let's make it consistent with\n the rest.\n\n When installed (see below) this command is part of the git suite,\n so the end blurb should say something to that effect.\n\nE.: \"make install\" will now install the \"scalar\" binary, but only if\n \"INSTALL_SCALAR\" is defined.\n\n Relative to our $(prefix) we'll now have (this is with\n INSTALL_SYMLINKS=Y):\n\n libexec/git-core/scalar: symbolic link to ../../bin/scalar\n bin/scalar: ELF 64-bit LSB pie executable,\n\n Putting it into libexec isn't strictly necessary, but as we do it\n with \"git\" we do that by default, and this will ensure that anyone\n who relies on their path being \"$(git --exec-path)\" will also be\n able to find \"scalar\".\n\n Perhaps we shouldn't put it in \"libexec\" at all, but for now let's\n just follow the herd.\n\nF.: Likewise \"make install-man install-doc install-html\" works, and\n will install \"scalar\" documentation if \"INSTALL_SCALAR\" is\n defined.\n\n We'll install these files with those targets (and correctly split\n them by target, e.g. only scalar.1 with \"install-man\"):\n\n share/doc/git-doc/scalar.txt: ASCII text\n share/doc/git-doc/scalar.html: XML 1.0 document, ASCII text, with CRLF line terminators\n share/man/man1/scalar.1: troff or preprocessor input, ASCII text, with very long lines\n\n== Fixed ==\n\nG.: The dependency graph of contrib/scalar/Makefile was broken when it\n came to scalar.o, it only depended on ../../libgit.a. So it was a\n requirement to run the top-level Makefile first to get around some\n of its dependency issues:\n\n make && make -C contrib/scalar\n\nH.: By having the top-level Makefile build scalar.o it'll work as well\n as any other git code, including benefiting from\n COMPUTE_HEADER_DEPENDENCIES.\n\n Targets such as (and possibly others):\n\n - \"make tags TAGS cscope\"\n - \"make coccicheck\"\n\t- \"make check-docs\"\n\n Wouldn't consider contrib/scalar at all, some of those would have\n been fixable, but as shown in the rest of this patch it's easier\n just to have scalar built like any other program instead.\n\n== Discussion and motivation ==\n\nI've been fixing various dependency issues in git's Makefile\ninfrastructure recently, some of which has been integrated into\n\"master\", some of which is currently under review on-list, and some of\nwhich isn't submitted yet.\n\nTo the extent that we have build dependency issues and cases where we\ncan't build something in parallel it's usually because we're invoking\none Makefile from another. That's the case in most of our\nsub-Makefiles, which will usually need to invoke or depend on\nsomething created in the top-level Makefile.\n\nNone of {t,Documentation,template}/Makefile etc. are truly\nindependent, as looking at $(git grep -F '../' '*Makefile') will\nreveal, and we'll sometimes need to duplicate logic between them\npurely to get around them not being driven by one top-level Makefile\nlogic.\n\nMuch has been written on this topic, likely most notably the\n\"Recursive Make Considered Harmful\" paper (full PDF link at [5]), and\nto be fair, the there's also a well-known \"Non-recursive Make\nConsidered Harmful\" retort at [6]).\n\nTheoretical opinions on the topic clearly differ. From a purely\npractical perspective I ran into textual and semantic conflicts with\nthe \"contrib/scalar/\" work.\n\nI think the diffstat of this patch demonstrates that this is a much\nsimpler approach, particularly considering that the code being\nreplaced didn't perform much of the needed integrations that are \"new\"\nhere. E.g. we now have \"make install\". The unsubmitted [7] (linked to\nby [8]) shows the boilerplate we'd need for that with the alternate\napproach.\n\nIt would be more pleasing to not have to hardcode \"git\" in the\npre-image and now \"git\" and \"scalar\" in the post-image in several\nplaces. My initial WIP [3] looks better, but built on a set of\nMakefile reorganization patches.\n\nI'd like to not make those patches a dependency of this change, but\nreaders should rest assured that we're really not ending up with as\nmany \"scalar\" special-cases in the long term as this diff indicates,\nmost or all of those can all be generalized into Makefile refactoring\nthat teaches our build system to build N number of differently named\ntop-level commands.\n\n1. https://lore.kernel.org/git/pull.1005.v6.git.1635323239.gitgitgadget@gmail.com/\n2. https://lore.kernel.org/git/87czpuxbwp.fsf@evledraar.gmail.com/\n3. https://lore.kernel.org/git/87ilz44kdk.fsf@evledraar.gmail.com/\n4. https://lore.kernel.org/git/b67bbef4-e4c3-b6a7-1c7f-7d405902ef8b@gmail.com/\n5. https://web.archive.org/web/20150330111905/http://miller.emu.id.au/pmiller/books/rmch/\n6. https://www.microsoft.com/en-us/research/wp-content/uploads/2016/03/hadrian.pdf\n7. https://github.com/dscho/git/commit/473ca8ae673\n8. https://lore.kernel.org/git/nycvar.QRO.7.76.6.2110210947590.56@tvgsbejvaqbjf.bet/\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n\nOn Wed, Oct 27 2021, Derrick Stolee wrote:\n\n> [...]\n> All of what I say is up for debate. I would love for others to contribute\n> options that I missed. I will send a reply to this message that summarizes\n> all options that are recommended in this thread, along with some attempt\n> to summarize any consensus that we might be approaching.\n\nI figured working code was better than continuing discussion on-list\nwith reference to working code. As noted in the patch itself htis goes\non top of Johannes's v6 scalar patches. CI run at :\nhttps://github.com/avar/git/actions/runs/1395993870\n\nTo the extent that the above commit message & below diff don't serve\nas a reply I'll reply to your E-Mail seperately.\n\nThanks a lot for getting the ball rolling on this.\n\n .gitignore | 1 +\n Documentation/Makefile | 4 +\n Documentation/cmd-list.perl | 2 +-\n Documentation/git.txt | 12 +++\n {contrib/scalar => Documentation}/scalar.txt | 4 +-\n Makefile | 96 ++++++++++++++------\n command-list.txt | 2 +\n contrib/scalar/.gitignore | 5 -\n contrib/scalar/Makefile | 57 ------------\n contrib/scalar/t/Makefile | 78 ----------------\n contrib/scalar/scalar.c => scalar.c | 0\n {contrib/scalar/t => t}/t9099-scalar.sh | 42 ++++++---\n 12 files changed, 115 insertions(+), 188 deletions(-)\n rename {contrib/scalar => Documentation}/scalar.txt (99%)\n delete mode 100644 contrib/scalar/.gitignore\n delete mode 100644 contrib/scalar/Makefile\n delete mode 100644 contrib/scalar/t/Makefile\n rename contrib/scalar/scalar.c => scalar.c (100%)\n rename {contrib/scalar/t => t}/t9099-scalar.sh (72%)\n\ndiff --git a/.gitignore b/.gitignore\nindex 054249b20a8..7f590115545 100644\n--- a/.gitignore\n+++ b/.gitignore\n@@ -216,6 +216,7 @@\n /configure\n /.vscode/\n /tags\n+/scalar\n /TAGS\n /cscope*\n /compile_commands.json\ndiff --git a/Documentation/Makefile b/Documentation/Makefile\nindex 911b6bf79c5..c017619ad2a 100644\n--- a/Documentation/Makefile\n+++ b/Documentation/Makefile\n@@ -18,6 +18,9 @@ MAN1_TXT += $(filter-out \\\n MAN1_TXT += git.txt\n MAN1_TXT += gitk.txt\n MAN1_TXT += gitweb.txt\n+ifndef NO_INSTALL_SCALAR_DOC\n+MAN1_TXT += scalar.txt\n+endif\n \n # man5 / man7 guides (note: new guides should also be added to command-list.txt)\n MAN5_TXT += gitattributes.txt\n@@ -311,6 +314,7 @@ endif\n cmds_txt = cmds-ancillaryinterrogators.txt \\\n \tcmds-ancillarymanipulators.txt \\\n \tcmds-mainporcelain.txt \\\n+\tcmds-optionalcontrib.txt \\\n \tcmds-plumbinginterrogators.txt \\\n \tcmds-plumbingmanipulators.txt \\\n \tcmds-synchingrepositories.txt \\\ndiff --git a/Documentation/cmd-list.perl b/Documentation/cmd-list.perl\nindex af5da45d287..0f4b1b23cfe 100755\n--- a/Documentation/cmd-list.perl\n+++ b/Documentation/cmd-list.perl\n@@ -10,7 +10,7 @@ sub format_one {\n \t$state = 0;\n \topen I, '<', \"$name.txt\" or die \"No such file $name.txt\";\n \twhile () {\n-\t\tif (/^git[a-z0-9-]*\\(([0-9])\\)$/) {\n+\t\tif (/^[a-z0-9-]*\\(([0-9])\\)$/) {\n \t\t\t$mansection = $1;\n \t\t\tnext;\n \t\t}\ndiff --git a/Documentation/git.txt b/Documentation/git.txt\nindex d63c65e67d8..d2b1280ff9d 100644\n--- a/Documentation/git.txt\n+++ b/Documentation/git.txt\n@@ -337,6 +337,18 @@ The following documentation pages are guides about Git concepts.\n \n include::cmds-guide.txt[]\n \n+Optional contrib commands\n+-------------------------\n+\n+The following commands are included with the git sources, but may not\n+be present in your installation.\n+\n+These should be considered \"contrib\"-level when it comes to\n+maintenance and stability promises. They might not even be included in\n+your installation, and may either drastically change in the future, or\n+go away entirely.\n+\n+include::cmds-optionalcontrib.txt[]\n \n Configuration Mechanism\n -----------------------\ndiff --git a/contrib/scalar/scalar.txt b/Documentation/scalar.txt\nsimilarity index 99%\nrename from contrib/scalar/scalar.txt\nrename to Documentation/scalar.txt\nindex cf4e5b889cc..197e9983a0f 100644\n--- a/contrib/scalar/scalar.txt\n+++ b/Documentation/scalar.txt\n@@ -150,6 +150,6 @@ SEE ALSO\n --------\n linkgit:git-clone[1], linkgit:git-maintenance[1].\n \n-Scalar\n+GIT\n ---\n-Associated with the linkgit:git[1] suite\n+Part of the linkgit:git[1] suite\ndiff --git a/Makefile b/Makefile\nindex 033a50a8b99..78da6c3661a 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -459,6 +459,11 @@ all::\n # INSTALL_STRIP can be set to \"-s\" to strip binaries during installation,\n # if your $(INSTALL) command supports the option.\n #\n+# Define INSTALL_SCALAR if you would like to install the optional\n+# \"scalar\" command. This command is considered \"contrib\"-level, see\n+# 'Optional \"contrib\" commands' in the built (with \"make man\") git(1)\n+# manual page.\n+#\n # Define GENERATE_COMPILATION_DATABASE to \"yes\" to generate JSON compilation\n # database entries during compilation if your compiler supports it, using the\n # `-MJ` flag. The JSON entries will be placed in the `compile_commands/`\n@@ -579,8 +584,10 @@ GIT_OBJS =\n LIB_OBJS =\n OBJECTS =\n PROGRAM_OBJS =\n+OTHER_PROGRAMS =\n PROGRAMS =\n EXCLUDED_PROGRAMS =\n+SCALAR_OBJS =\n SCRIPT_PERL =\n SCRIPT_PYTHON =\n SCRIPT_SH =\n@@ -784,7 +791,8 @@ BUILT_INS += git-switch$X\n BUILT_INS += git-whatchanged$X\n \n # what 'all' will build but not install in gitexecdir\n-OTHER_PROGRAMS = git$X\n+OTHER_PROGRAMS += git$X\n+OTHER_PROGRAMS += scalar$X\n \n # what test wrappers are needed and 'install' will install, in bindir\n BINDIR_PROGRAMS_NEED_X += git\n@@ -793,6 +801,18 @@ BINDIR_PROGRAMS_NEED_X += git-shell\n BINDIR_PROGRAMS_NEED_X += git-upload-archive\n BINDIR_PROGRAMS_NEED_X += git-upload-pack\n \n+# Sometimes we only have a test wrapper, but not a program to\n+# install. This isn't so pretty, and we could refactor the\n+# bin-wrappers/% and install code to make it unnecessary.\n+ifdef INSTALL_SCALAR\n+PROGRAMS += scalar$X\n+BINDIR_PROGRAMS_NEED_X += scalar\n+endif\n+TEST_BINDIR_PROGRAMS_NEED_X = $(BINDIR_PROGRAMS_NEED_X)\n+ifndef INSTALL_SCALAR\n+TEST_BINDIR_PROGRAMS_NEED_X += scalar\n+endif\n+\n BINDIR_PROGRAMS_NO_X += git-cvsserver\n \n # Set paths to tools early so that they can be used for version tests.\n@@ -2210,9 +2230,13 @@ git.sp git.s git.o: EXTRA_CPPFLAGS = \\\n \t'-DGIT_MAN_PATH=\"$(mandir_relative_SQ)\"' \\\n \t'-DGIT_INFO_PATH=\"$(infodir_relative_SQ)\"'\n \n-git$X: git.o GIT-LDFLAGS $(BUILTIN_OBJS) $(GITLIBS)\n-\t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) \\\n-\t\t$(filter %.o,$^) $(LIBS)\n+define top-level-cmd-rule\n+$(1)$X: $(1).o GIT-LDFLAGS $$(GITLIBS) $(2)\n+\t$$(QUIET_LINK)$$(CC) $$(ALL_CFLAGS) -o $$@ $$(ALL_LDFLAGS) \\\n+\t\t$$(filter %.o,$$^) $(LIBS)\n+endef\n+$(eval $(call top-level-cmd-rule,git,$(BUILTIN_OBJS)))\n+$(eval $(call top-level-cmd-rule,scalar,))\n \n help.sp help.s help.o: command-list.h\n hook.sp hook.s hook.o: hook-list.h\n@@ -2448,7 +2472,12 @@ GIT_OBJS += git.o\n .PHONY: git-objs\n git-objs: $(GIT_OBJS)\n \n+SCALAR_OBJS += scalar.o\n+.PHONY: scalar-objs\n+scalar-objs: $(SCALAR_OBJS)\n+\n OBJECTS += $(GIT_OBJS)\n+OBJECTS += $(SCALAR_OBJS)\n OBJECTS += $(PROGRAM_OBJS)\n OBJECTS += $(TEST_OBJS)\n OBJECTS += $(XDIFF_OBJS)\n@@ -2457,10 +2486,6 @@ ifndef NO_CURL\n \tOBJECTS += http.o http-walker.o remote-curl.o\n endif\n \n-SCALAR_SOURCES := contrib/scalar/scalar.c\n-SCALAR_OBJECTS := $(SCALAR_SOURCES:c=o)\n-OBJECTS += $(SCALAR_OBJECTS)\n-\n .PHONY: objects\n objects: $(OBJECTS)\n \n@@ -2594,10 +2619,6 @@ $(REMOTE_CURL_PRIMARY): remote-curl.o http.o http-walker.o GIT-LDFLAGS $(GITLIBS\n \t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \\\n \t\t$(CURL_LIBCURL) $(EXPAT_LIBEXPAT) $(LIBS)\n \n-contrib/scalar/scalar$X: $(SCALAR_OBJECTS) GIT-LDFLAGS $(GITLIBS)\n-\t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) \\\n-\t\t$(filter %.o,$^) $(LIBS)\n-\n $(LIB_FILE): $(LIB_OBJS)\n \t$(QUIET_AR)$(RM) $@ && $(AR) $(ARFLAGS) $@ $^\n \n@@ -2870,15 +2891,18 @@ GIT-PYTHON-VARS: FORCE\n fi\n endif\n \n-test_bindir_programs := $(patsubst %,bin-wrappers/%,$(BINDIR_PROGRAMS_NEED_X) $(BINDIR_PROGRAMS_NO_X) $(TEST_PROGRAMS_NEED_X))\n+test_bindir_programs := $(patsubst %,bin-wrappers/%,$(TEST_BINDIR_PROGRAMS_NEED_X) $(BINDIR_PROGRAMS_NO_X) $(TEST_PROGRAMS_NEED_X))\n \n all:: $(TEST_PROGRAMS) $(test_bindir_programs)\n \n+# Substitution that's only done on programs named git-* in\n+# bin-wrappers/*\n+GIT_TEST_BINDIR_PROGRAMS_NEED_X = $(filter-out scalar,$(TEST_BINDIR_PROGRAMS_NEED_X))\n bin-wrappers/%: wrap-for-bin.sh\n \t@mkdir -p bin-wrappers\n \t$(QUIET_GEN)sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \\\n \t -e 's|@@BUILD_DIR@@|$(shell pwd)|' \\\n-\t -e 's|@@PROG@@|$(patsubst test-%,t/helper/test-%$(X),$(@F))$(patsubst git%,$(X),$(filter $(@F),$(BINDIR_PROGRAMS_NEED_X)))|' < $< > $@ && \\\n+\t -e 's|@@PROG@@|$(patsubst test-%,t/helper/test-%$(X),$(@F))$(patsubst git%,$(X),$(filter $(@F),$(GIT_TEST_BINDIR_PROGRAMS_NEED_X)))|' < $< > $@ && \\\n \tchmod +x $@\n \n # GNU make supports exporting all variables by \"export\" without parameters.\n@@ -3113,25 +3137,26 @@ endif\n install-gitweb:\n \t$(MAKE) -C gitweb install\n \n-install-doc: install-man-perl\n-\t$(MAKE) -C Documentation install\n-\n-install-man: install-man-perl\n-\t$(MAKE) -C Documentation install-man\n+ifdef INSTALL_SCALAR\n+NO_INSTALL_SCALAR_DOC =\n+else\n+NO_INSTALL_SCALAR_DOC = NoScalarPlease\n+endif\n \n install-man-perl: man-perl\n \t$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(mandir_SQ)/man3'\n \t(cd perl/build/man/man3 && $(TAR) cf - .) | \\\n \t(cd '$(DESTDIR_SQ)$(mandir_SQ)/man3' && umask 022 && $(TAR) xof -)\n \n-install-html:\n-\t$(MAKE) -C Documentation install-html\n-\n-install-info:\n-\t$(MAKE) -C Documentation install-info\n-\n-install-pdf:\n-\t$(MAKE) -C Documentation install-pdf\n+define install-doc-rule\n+$(1): $(3)\n+\t$$(MAKE) -C Documentation $(2) NO_INSTALL_SCALAR_DOC=$$(NO_INSTALL_SCALAR_DOC)\n+endef\n+$(eval $(call install-doc-rule,install-doc,install,install-man-perl))\n+$(eval $(call install-doc-rule,install-man,install-man,install-man-perl))\n+$(eval $(call install-doc-rule,install-html,install-html,))\n+$(eval $(call install-doc-rule,install-info,install-info,))\n+$(eval $(call install-doc-rule,install-pdf,install-pdf,))\n \n quick-install-doc:\n \t$(MAKE) -C Documentation quick-install\n@@ -3183,8 +3208,19 @@ ifneq ($(INCLUDE_DLLS_IN_ARTIFACTS),)\n OTHER_PROGRAMS += $(shell echo *.dll t/helper/*.dll)\n endif\n \n-artifacts-tar:: $(ALL_COMMANDS_TO_INSTALL) $(SCRIPT_LIB) $(OTHER_PROGRAMS) \\\n-\t\tGIT-BUILD-OPTIONS $(TEST_PROGRAMS) $(test_bindir_programs) \\\n+# No scalar in \"cmake\" yet, and since 4c2c38e800f (ci: modification of\n+# main.yml to use cmake for vs-build job, 2020-06-26) the \"vs-build\"\n+# job has a hard dependency on it. Let's fail in the tests instead of\n+# in the \"vs-build\" job itself.\n+ifeq ($(wildcard scalar),scalar)\n+ARCHIVE_OTHER_PROGRAMS = $(OTHER_PROGRAMS)\n+ARCHIVE_TEST_BINDIR_PROGRAMS = $(test_bindir_programs)\n+else\n+ARCHIVE_OTHER_PROGRAMS = $(filter-out scalar%, $(OTHER_PROGRAMS))\n+ARCHIVE_TEST_BINDIR_PROGRAMS = $(filter-out bin-wrappers/scalar,$(test_bindir_programs))\n+endif\n+artifacts-tar:: $(ALL_COMMANDS_TO_INSTALL) $(SCRIPT_LIB) $(ARCHIVE_OTHER_PROGRAMS) \\\n+\t\tGIT-BUILD-OPTIONS $(TEST_PROGRAMS) $(ARCHIVE_TEST_BINDIR_PROGRAMS) \\\n \t\t$(MOFILES)\n \t$(QUIET_SUBDIR0)templates $(QUIET_SUBDIR1) \\\n \t\tSHELL_PATH='$(SHELL_PATH_SQ)' PERL_PATH='$(PERL_PATH_SQ)'\n@@ -3327,7 +3363,7 @@ check-docs::\n \t\t -e 's/\\.txt//'; \\\n \t) | while read how cmd; \\\n \tdo \\\n-\t\tcase \" $(patsubst %$X,%,$(ALL_COMMANDS) $(BUILT_INS) $(EXCLUDED_PROGRAMS)) \" in \\\n+\t\tcase \" $(patsubst %$X,%,$(ALL_COMMANDS) $(BUILT_INS) $(EXCLUDED_PROGRAMS) scalar) \" in \\\n \t\t*\" $$cmd \"*)\t;; \\\n \t\t*) echo \"removed but $$how: $$cmd\" ;; \\\n \t\tesac; \\\ndiff --git a/command-list.txt b/command-list.txt\nindex a289f09ed6f..350f5aec074 100644\n--- a/command-list.txt\n+++ b/command-list.txt\n@@ -16,6 +16,7 @@\n # synchingrepositories\n # synchelpers\n # purehelpers\n+# optionalcontrib\n #\n # The type names are self explanatory. But if you want to see what\n # command belongs to what group to get a better picture, have a look\n@@ -214,3 +215,4 @@ gitsubmodules guide\n gittutorial-2 guide\n gittutorial guide\n gitworkflows guide\n+scalar optionalcontrib\ndiff --git a/contrib/scalar/.gitignore b/contrib/scalar/.gitignore\ndeleted file mode 100644\nindex 00441073f59..00000000000\n--- a/contrib/scalar/.gitignore\n+++ /dev/null\n@@ -1,5 +0,0 @@\n-/*.xml\n-/*.1\n-/*.html\n-/*.exe\n-/scalar\ndiff --git a/contrib/scalar/Makefile b/contrib/scalar/Makefile\ndeleted file mode 100644\nindex 44796572ef4..00000000000\n--- a/contrib/scalar/Makefile\n+++ /dev/null\n@@ -1,57 +0,0 @@\n-QUIET_SUBDIR0 = +$(MAKE) -C # space to separate -C and subdir\n-QUIET_SUBDIR1 =\n-\n-ifneq ($(findstring s,$(MAKEFLAGS)),s)\n-ifndef V\n-\tQUIET_GEN = @echo ' ' GEN $@;\n-\tQUIET_SUBDIR0 = +@subdir=\n-\tQUIET_SUBDIR1 = ;$(NO_SUBDIR) echo ' ' SUBDIR $$subdir; \\\n-\t\t\t $(MAKE) $(PRINT_DIR) -C $$subdir\n-\tQUIET = @\n-else\n-\texport V\n-endif\n-endif\n-\n-all:\n-\n-include ../../config.mak.uname\n--include ../../config.mak.autogen\n--include ../../config.mak\n-\n-TARGETS = scalar$(X) scalar.o\n-GITLIBS = ../../common-main.o ../../libgit.a ../../xdiff/lib.a\n-\n-all: scalar$(X) ../../bin-wrappers/scalar\n-\n-$(GITLIBS):\n-\t$(QUIET_SUBDIR0)../.. $(QUIET_SUBDIR1) $(subst ../../,,$@)\n-\n-$(TARGETS): $(GITLIBS) scalar.c\n-\t$(QUIET_SUBDIR0)../.. $(QUIET_SUBDIR1) $(patsubst %,contrib/scalar/%,$@)\n-\n-clean:\n-\t$(RM) $(TARGETS) ../../bin-wrappers/scalar\n-\t$(RM) scalar.1 scalar.html scalar.xml\n-\n-../../bin-wrappers/scalar: ../../wrap-for-bin.sh Makefile\n-\t@mkdir -p ../../bin-wrappers\n-\t$(QUIET_GEN)sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \\\n-\t -e 's|@@BUILD_DIR@@|$(shell cd ../.. && pwd)|' \\\n-\t -e 's|@@PROG@@|contrib/scalar/scalar$(X)|' < $< > $@ && \\\n-\tchmod +x $@\n-\n-test: all\n-\t$(MAKE) -C t\n-\n-docs: scalar.html scalar.1\n-\n-scalar.html: | scalar.1 # prevent them from trying to build `doc.dep` in parallel\n-\n-scalar.html scalar.1: scalar.txt\n-\t$(QUIET_SUBDIR0)../../Documentation$(QUIET_SUBDIR1) \\\n-\t\tMAN_TXT=../contrib/scalar/scalar.txt \\\n-\t\t../contrib/scalar/$@\n-\t$(QUIET)test scalar.1 != \"$@\" || mv ../../Documentation/$@ .\n-\n-.PHONY: $(GITLIBS) all clean docs test FORCE\ndiff --git a/contrib/scalar/t/Makefile b/contrib/scalar/t/Makefile\ndeleted file mode 100644\nindex 6170672bb37..00000000000\n--- a/contrib/scalar/t/Makefile\n+++ /dev/null\n@@ -1,78 +0,0 @@\n-# Run scalar tests\n-#\n-# Copyright (c) 2005,2021 Junio C Hamano, Johannes Schindelin\n-#\n-\n--include ../../../config.mak.autogen\n--include ../../../config.mak\n-\n-SHELL_PATH ?= $(SHELL)\n-PERL_PATH ?= /usr/bin/perl\n-RM ?= rm -f\n-PROVE ?= prove\n-DEFAULT_TEST_TARGET ?= test\n-TEST_LINT ?= test-lint\n-\n-ifdef TEST_OUTPUT_DIRECTORY\n-TEST_RESULTS_DIRECTORY = $(TEST_OUTPUT_DIRECTORY)/test-results\n-else\n-TEST_RESULTS_DIRECTORY = ../../../t/test-results\n-endif\n-\n-# Shell quote;\n-SHELL_PATH_SQ = $(subst ','\\'',$(SHELL_PATH))\n-PERL_PATH_SQ = $(subst ','\\'',$(PERL_PATH))\n-TEST_RESULTS_DIRECTORY_SQ = $(subst ','\\'',$(TEST_RESULTS_DIRECTORY))\n-\n-T = $(sort $(wildcard t[0-9][0-9][0-9][0-9]-*.sh))\n-\n-all: $(DEFAULT_TEST_TARGET)\n-\n-test: $(TEST_LINT)\n-\t$(MAKE) aggregate-results-and-cleanup\n-\n-prove: $(TEST_LINT)\n-\t@echo \"*** prove ***\"; GIT_CONFIG=.git/config $(PROVE) --exec '$(SHELL_PATH_SQ)' $(GIT_PROVE_OPTS) $(T) :: $(GIT_TEST_OPTS)\n-\t$(MAKE) clean-except-prove-cache\n-\n-$(T):\n-\t@echo \"*** $@ ***\"; GIT_CONFIG=.git/config '$(SHELL_PATH_SQ)' $@ $(GIT_TEST_OPTS)\n-\n-clean-except-prove-cache:\n-\t$(RM) -r 'trash directory'.* '$(TEST_RESULTS_DIRECTORY_SQ)'\n-\t$(RM) -r valgrind/bin\n-\n-clean: clean-except-prove-cache\n-\t$(RM) .prove\n-\n-test-lint: test-lint-duplicates test-lint-executable test-lint-shell-syntax\n-\n-test-lint-duplicates:\n-\t@dups=`echo $(T) | tr ' ' '\\n' | sed 's/-.*//' | sort | uniq -d` && \\\n-\t\ttest -z \"$$dups\" || { \\\n-\t\techo >&2 \"duplicate test numbers:\" $$dups; exit 1; }\n-\n-test-lint-executable:\n-\t@bad=`for i in $(T); do test -x \"$$i\" || echo $$i; done` && \\\n-\t\ttest -z \"$$bad\" || { \\\n-\t\techo >&2 \"non-executable tests:\" $$bad; exit 1; }\n-\n-test-lint-shell-syntax:\n-\t@'$(PERL_PATH_SQ)' ../../../t/check-non-portable-shell.pl $(T)\n-\n-aggregate-results-and-cleanup: $(T)\n-\t$(MAKE) aggregate-results\n-\t$(MAKE) clean\n-\n-aggregate-results:\n-\tfor f in '$(TEST_RESULTS_DIRECTORY_SQ)'/t*-*.counts; do \\\n-\t\techo \"$$f\"; \\\n-\tdone | '$(SHELL_PATH_SQ)' ../../../t/aggregate-results.sh\n-\n-valgrind:\n-\t$(MAKE) GIT_TEST_OPTS=\"$(GIT_TEST_OPTS) --valgrind\"\n-\n-test-results:\n-\tmkdir -p test-results\n-\n-.PHONY: $(T) aggregate-results clean valgrind\ndiff --git a/contrib/scalar/scalar.c b/scalar.c\nsimilarity index 100%\nrename from contrib/scalar/scalar.c\nrename to scalar.c\ndiff --git a/contrib/scalar/t/t9099-scalar.sh b/t/t9099-scalar.sh\nsimilarity index 72%\nrename from contrib/scalar/t/t9099-scalar.sh\nrename to t/t9099-scalar.sh\nindex 7e8771d0eff..788695d857e 100755\n--- a/contrib/scalar/t/t9099-scalar.sh\n+++ b/t/t9099-scalar.sh\n@@ -2,13 +2,13 @@\n \n test_description='test the `scalar` command'\n \n-TEST_DIRECTORY=$PWD/../../../t\n-export TEST_DIRECTORY\n+. ./test-lib.sh\n \n-# Make it work with --no-bin-wrappers\n-PATH=$PWD/..:$PATH\n-\n-. ../../../t/test-lib.sh\n+if test_have_prereq WINDOWS && ! scalar list 2>/dev/null\n+then\n+\tskip_all='the contrib cmake process does not build \"scalar\" yet'\n+\ttest_done\n+fi\n \n GIT_TEST_MAINT_SCHEDULER=\"crontab:test-tool crontab ../cron.txt\"\n export GIT_TEST_MAINT_SCHEDULER\n@@ -17,7 +17,23 @@ test_expect_success 'scalar shows a usage' '\n \ttest_expect_code 129 scalar -h\n '\n \n-test_expect_success 'scalar unregister' '\n+test_expect_success 'setup' '\n+\ttest_commit first &&\n+\ttest_commit second &&\n+\ttest_commit third &&\n+\tgit switch -c parallel first\n+'\n+\n+test_lazy_prereq SCALAR_REGISTER '\n+\tgit init test/src &&\n+\tscalar register test/src &&\n+\tscalar list >expect &&\n+\tscalar unregister test/src &&\n+\t(cd test/src && echo \"$PWD\") >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success SCALAR_REGISTER 'scalar unregister' '\n \tgit init vanish/src &&\n \tscalar register vanish/src &&\n \tgit config --get --global --fixed-value \\\n@@ -32,18 +48,14 @@ test_expect_success 'scalar unregister' '\n \t! grep -F \"$(pwd)/vanish/src\" scalar.repos\n '\n \n-test_expect_success 'set up repository to clone' '\n-\ttest_commit first &&\n-\ttest_commit second &&\n-\ttest_commit third &&\n-\tgit switch -c parallel first &&\n+test_expect_success SCALAR_REGISTER 'set up repository to clone' '\n \tmkdir -p 1/2 &&\n \ttest_commit 1/2/3 &&\n \tgit config uploadPack.allowFilter true &&\n \tgit config uploadPack.allowAnySHA1InWant true\n '\n \n-test_expect_success 'scalar clone' '\n+test_expect_success SCALAR_REGISTER 'scalar clone' '\n \tsecond=$(git rev-parse --verify second:second.t) &&\n \tscalar clone \"file://$(pwd)\" cloned --single-branch &&\n \t(\n@@ -65,7 +77,7 @@ test_expect_success 'scalar clone' '\n \t)\n '\n \n-test_expect_success 'scalar reconfigure' '\n+test_expect_success SCALAR_REGISTER 'scalar reconfigure' '\n \tgit init one/src &&\n \tscalar register one &&\n \tgit -C one/src config core.preloadIndex false &&\n@@ -80,7 +92,7 @@ test_expect_success 'scalar delete without enlistment shows a usage' '\n \ttest_expect_code 129 scalar delete\n '\n \n-test_expect_success 'scalar delete with enlistment' '\n+test_expect_success SCALAR_REGISTER 'scalar delete with enlistment' '\n \tscalar delete cloned &&\n \ttest_path_is_missing cloned\n '\n-- \n2.33.1.1572.g4991fc4c1bd\n\n"},{"id":"439988","messageId":"211029.86k0hww317.gmgdl@evledraar.gmail.com","threadId":"56797","inReplyTo":"b67bbef4-e4c3-b6a7-1c7f-7d405902ef8b@gmail.com","subject":"Re: [Discussion] The architecture of Scalar (and others) within Git","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-28T18:58:15Z","receivedAt":"2021-10-29T10:15:06Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Oct 27 2021, Derrick Stolee wrote:\n\n> I'm starting this discussion thread to create a new area to consider these\n> high-level concepts. Specifically: how should a new component like Scalar\n> be included in the Git codebase?\n\nFor context. I've submitted a PATCH form of the back & forth\nyou/me/Johannes have been having on this topic, which hopefully should\nunambiguously clear up exactly what it is that I've been suggesting:\nhttps://lore.kernel.org/git/patch-1.1-86fb8d56307-20211028T185016Z-avarab@gmail.com/\n\nAs noted there some inline comments here, to the extent that it's not\naddressed by the patch in some way.\n\n> Options for preferred end state\n> -------------------------------\n>\n> Let's get into some concrete proposals for the location of the Scalar CLI\n> within the Git codebase. These are ordered based on increasing\n> responsibility on the Git community: the first option should minimize\n> community responsibility and maximize responsibility on the Scalar\n> maintainers. We will discuss the pros and cons of each option after fully\n> describing each of them.\n>\n> (Please add any options that I may have missed!)\n\nI think what I'd like to add is...[continued below]...\n\n> **Option 1:** One-directional coupling in `contrib/scalar/`\n>\n> The Scalar CLI exists entirely within `contrib/scalar/`, including all code\n> and test infrastructure. To opt-in to building and testing Scalar, run\n> `make -C contrib/scalar test` to build Git and Scalar and run the Scalar\n> test scripts.\n\nAside: I think anyone reading this summary might assume that this 1st\nitem describes the current state of the scalar patches on-list, but\nthat's not the case.\n\nI.e. that coupling is thoroughly bi-directional in its integration, and\nthe code is built by default.\n\n> [...]\n> **Option 2:** Loose coupling in a new scalar/ directory\n> [...]\n> **Option 3:** Tight coupling with entire Git project\n> [...]\n\n...[continued from above]...an \"Option 0: Do we care?\"\n\nI think I've been the only one who's been pointing out anything about\nthe location of these source files, which prompted this mail. But it's\nnot because I care about the location per-se.\n\nI don't think it matters, and I think arguing about that per-se would\njust amount to needless bikeshedding.\n\nWhat I *have* been pointing out is to the effect of \"hey, your\nbuild/test integration is broken in ways, it looks to me like the\neasiest way out of that happens to be to move these files to the\ntop-level\".\n\nE.g. the scalar series is now at v6, but apparently it hasn't been\nnoticed that some of the tests were broken on OSX due to apparent errors\nwith launchctl integration. That's because the patches had the code\ncompiled, but didn't run the tests.\n\nIf those and other issues with the series were fixed in a way that puts\nit at whatever in-tree path you & Johannes would prefer I wouldn't mind\nat all, and I doubt anyone else would have much of a strong preference\neither.\n\nI think the main reason there's been resistance from you two to consider\nmoving it away from \"contrib/*\" (but correct me if I'm wrong, I'm still\nunclear on this point, even after reading this E-Mail in full) is that\nyou think it's important to communicate a certain maintenance status for\n\"scalar\", so that we can freely change it, and even \"git rm\" it one day\nwhen it's served its purpose.\n\nI think the patch I've linked to above should address that in a way\nthat's primarily aimed at communicating that to users, not just git.git\ndevelopers. I.e. we now have it listed in \"man git\" under a \"Optional\ncontrib commands\" section that says:\n \n The following commands are included with the git sources, but may\n not be present in your installation.\n \n These should be considered \"contrib\"-level when it comes to\n maintenance and stability promises. They might not even be included\n in your installation, and may either drastically change in the\n future, or go away entirely.\n\nSo I guess my preferred option is: \"Option 0: Make it integrate with\ngit.git in a way that's not buggy\". I believe the patch I've submitted\ngets us to that state.\n\nIf you wanted to run with that & move it around in-tree I think that\nwould be fine, as long as we're not introducing bugs by doing so.\n\nSome specific & briefer comments below:\n\n> [...]\n> **Option 1:** One-directional coupling in `contrib/scalar/`\n>\n> This option was our initial choice because it minimizes the responsibility\n> of the Git community. While `contrib/scalar/scalar.c` depends on code in\n> `libgit.a`, the implementation does not require that code to change to\n> accommodate the needs of Scalar. The test suite and documentation is\n> separate.\n>\n> This does mean that changes to `libgit.a` could break Scalar without any\n> feedback to the developer that has not compiled Scalar. The Scalar\n> maintainers would then need to watch for this and send separate updates to\n> Scalar that fix these dependency breaks. This reduces developer friction,\n> but might cause some extra burden on the Git maintainer. If these \"catch\n> up to dependency breaks\" updates happen only after a release candidate is\n> out, then maybe this isn't too much of a burden. We don't typically have\n> release candidates for minor releases, so there is some risk there that\n> minor releases could include breaks.\n>\n> If we make our CI builds include Scalar by default, then the previous\n> paragraph about developer friction is negated.\n\nWithout going into exhaustive detail I'll just note as above that much\nof this describes a state that's got little or nothing to do with the\ncontrib/scalar/* patches on-list, which are thoroughly\nbi-bidirectionally integrated, build (but don't test) by default etc.\n\n> Having Scalar in `contrib/` makes it easy to differentiate it as an\n> optional component that distributors could choose to include or leave out.\n\nWhy would the FS path have anything to do with making that easy?\nE.g. distributors now can:\n\n make install NO_REGEX=MineIsBroken\n\nThat we store the relevant sources in-tree at compat/regex/ doesn't\nmatter to distributors. We could just as well have them at the\ntop-level. The UI we present is just some Makefile knob.\n\nYes, the currently propoed scalar patches would do the same via a\nsub-Makefile, and e.g. contrib/subtree works that way, but does it\n*need* to be like that for distributers? No, they'll just read the\nrelease notes, and copy whatever the relevant instructions are.\n\n> Code in `contrib/` has a lower barrier to entry. In particular, the Scalar\n> CLI is not intended to be up for debate for historical reasons. If we make\n> an exception for Scalar but want Scalar integrated outside of `contrib/`,\n> then are we setting expectations for other tools that might want to be\n> included?\n\nWe're not. We can just document the status of individual logical\ncomponents, separate from in-tree FS paths.\n\n> However, projects within `contrib/` do not currently depend on `libgit.a`\n> the way Scalar does. (This is not historically true, as an older project\n> ad such dependencies, but has since been ejected.).[...]\n\nWell, we've got contrib/buildsystems/, if a thing that builds libgit.a\ndoesn't have an (inter-)dependency on libgit.a I don't know what would\n:)\n\nBut yes, we've got no C code in contrib/ currently that links *to*\nlibgit.a, which is one aspect of the breakages I've pointed out with the\nscalar patches.\n\nBut as contrib/buildsystems/ shows the \"contrib/\" directory is not some\n\"hands-off\" directory for git.git developers, i.e. if you make certain\nchanges in non-contrib the vs-build CI will break, since it has a hard\ndependency on that contrib component.\n\nSo in terms of placing things in that directory unambiguously\ncommunicating some \"third party\" or \"top-level doesn't depend on this\"\nintent or state, I don't think it's really doing the work you seem to\nthink it will.\n\n> This option explicitly mentions that all knowledge of how to build Scalar\n> lives within `contrib/scalar/` to avoid disrupting the core `Makefile`.\n> This has already led to debate about some duplication in the Scalar\n> `Makefile` and the one at root.\n\nAs the instigator of particularl that debate: No that's not accurate at\nall.\n\nWell, to be fair I haven't been commenting on some hypothetical\n\"contrib/scalar/\" that's \"one-directional\" and that you \"opt-in to\nbuilding\". \n\nAs noted above I'm rather confused by this \"contrib/scalar/\" summary\ndescribing something that doesn't at all map unto the relevant posted\npatches.\n\nTo the extent that I have been discussing this aspect of the actual\nposted patches it's been because the opposite of \"all knowledge of how\nto build Scalar lives within `contrib/scalar`\" is true, as a look at the\nremoved lines in the patch I've posted above shows.\n\nMost of that build and test infrastructure has a thoroughly hard\ndependency on Makefiles at the top-level, in a way that causes more work\nfor anyone who's changing any of that top-level build logic, not less.\n\n> **Option 2:** Loose coupling in a `scalar/` directory\n>\n> The first issue with this option is that the Scalar CLI is established and\n> is not up for modification, yet we would be contributing it in a location\n> that is typically under high scrutiny for things like this.\n\nI don't think anyone's been disputing the fundimental point that the\nwhole reason for having this in-tree scalar is for the convenience of\nsome existing users, and that any proposed changes to the CLI UI need to\nkeep that in mind.\n\nHow would just having it in some subdirectory change any of that? Sorry,\nI just continue not to understand what a \"contrib/scalar/\", \"scalar/\" or\nwhatever prefix would have anything to do with that.\n\n> This option also breaks into new territory, because even `git-gui` and\n> `gitk-git` are not based on C and do not depend on `libgit.a`.\n\nIf we're running with the idea that a one-level subdirectory of\nnot-really-git.git sources is somehow magical then in-tree \"sha1dc/\"\nqualifies for the first part of that.\n\nI.e. we build it by default, it's C based, but it doesn't depend on\nlibgit.a.\n\n> [...]\n> Developer friction increases as changes to `libgit.a` that break Scalar\n> should be fixed within the patches that cause those changes.\n\nSo again on the \"hypothetical contrib/scalar/*\" point: This is also true\nof Johannes's \"contrib/scalar/*\" patches on-list.\n\nSo having read this far this \"scalar/*\" distinction here seems to really\nbe describing something like the current state (but under a different\npath), and patches for \"Option 1\" have not been seen on-list.\n\n> [...]\n> **Option 3:** Tight coupling with the entire Git project\n>\n> This option removes the ability to opt-out of building the Scalar CLI.\n> Distributors might need to react to remove the `scalar` executable if they\n> do not want to include it.\n\nI'm apathetic to whether we actually install it by default, but in my\npatch above that's optional.\n\nSo \"tight coupling\" in the sense of not only building it, but also\nrunning the tests does not need to entail that we install it by default.\n\n> This option sets a risky precedent that new tools can be added to Git\n> without a rigorous review of the CLI.\n\nI think the changes I had to \"man git\" should cover this, not that the\nscalar CLI couldn't do with a bit more review/polishing (some of which I\ndiscovered recently when trying to get the CI working).\n\n> **Phase 1.** Keep code in contrib/ while we contribute the Scalar features.\n>\n> Since the current patches on the mailing list are not feature complete, it\n> can be beneficial to move forward with the code in `contrib/scalar/` until\n> we reach feature parity. At that point, we could move the source into its\n> final resting place.\n\nWhy specifically does it need to be at a different in-tree path before\nit reaches feature parity?\n\nThis proposal seems to implicitly assume a lot about the necessity for\nFS-path-based lifecycle management of components, without really\njustifying why any of that's needed.\n\nLet's just put things in their \"final place\", document their current\nstate for human consumption somewhere (commit messages, \"man git\", etc.)\nand avoid the path-based churn? Why isn't that OK?\n\n> [...]\n> Depending on the final goal, we could drop some of the work that is currently\n> built within the `contrib/scalar/` directory, such as `-c`/`-C` parsing or\n> documentation builds. These features would be reimplemented in the new\n> location, so we can prevent that duplicate effort if we have a different\n> final location in mind.\n\nJust on -c & -C parsing: It's already true that scalar uses libgit.a,\nit's even true that scalar (admittedly small) parts of libgit.a that no\nother caller uses directly.\n\nSo I don't see why having it in a given in-tree location and already\nneeding parts of libgit.a would preclude us from creating APIs\nspecifically for its use.\n\nParticularly in the case of git.c where those APIs would benefit any\nfuture \"top-level program sort of like a git(1)\" caller, and would/could\neven benefit our own sub-commands such as \"git commit-graph\" et al.\n\nE.g. if we lifted out the typo correction cmd dispatching from git.git\nand could use it for those + scalar.c, that would be very nice for\nexisting in-tree users, and would also reduce code duplication in\nscalar.c.\n\n> [...]\n> Thanks for your attention to this topic. I look forward to any new \"big\"\n> ideas in this space.\n\nI think that in this case a series of \"small\" ideas might cause any\npreconceived \"big\" solutions to evaporate once implemented.\n\nE.g. in [1] Johannes noted:\n\n \"Just forget about Scalar's build process. Forget about getting its\n CI to work. I have all that figured out already. It is all working\n as well as needed.\"[1]\n\nThat \"as well as needed\" is doing a lot of work, as noted above it was\nclearly broken on the OSX CI at least.\n\nOne thing my linked-to does is to fix/change that small aspect, does\nanyone mind the incremental step of scalar.c not only being built but\nalso tested? If so let them just note their concerns in the releant\nthread.\n\nOnce we address such point-by-point issues will any \"big\" ones remain?\nMaybe, maybe not.\n\n1. https://lore.kernel.org/git/nycvar.QRO.7.76.6.2110221734530.62@tvgsbejvaqbjf.bet/\n"},{"id":"440278","messageId":"xmqqbl33a2fr.fsf@gitster.g","threadId":"56797","inReplyTo":"b67bbef4-e4c3-b6a7-1c7f-7d405902ef8b@gmail.com","subject":"Re: [Discussion] The architecture of Scalar (and others) within Git","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-11-01T23:20:08Z","receivedAt":"2021-11-01T23:20:18Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Derrick Stolee writes:\n\n> Optimization Factors\n> --------------------\n>\n> There are multiple factors involved that we wish to optimize. Here is my\n> understanding of the most-important factors and how they interact:\n>\n> ### Value\n>\n> The main reason to include Scalar within the Git codebase is to make it\n> available to more users. This should have downstream effects based on user\n> feedback about what they like or dislike about the Scalar CLI that can\n> inform future features in core Git that simplify repository management at\n> scale.\n\nIn other words, Scalar wants to piggyback on Git's popularity, yet\nwants to retain total control of how its end-user experience should\nlook like, eating the cake and having it at the same time?\n\nI do not think it is fair to proceed with this discussion without\nhaving \"Scalar lives in its own tree, uses this mailing list to\ndiscuss how it should evolve, drawing expertise from the\ncontributors found here, but otherwise would stay to be a spearate\nproject\" as another option and debating its pros-and-cons, next to\nthe spectrum among contrib/scalar, scalar/, and scalar.c approaches.\nIIUC, cgit has one-directional coupling to depend on libgit.a just\nlike the Scalar wants to link with it---I am not saying Scalar should\ndo the same, but just raising it as an example that you do *not* have\nto live in my tree anywhere to be able to link with libgit.a; that\nwould probably count as an \"**Option 0:** A separate project that\nborrows from Git by having it as a submodule\".\n\nI can think of only one obvious advantage going that route, and that\nis why I said \"I am not saying Scalar should do the same\", which is\nthat the Scalar project will then retain total control over how it\nis built and how its end-user experience should stay the same.\n\nI do not think it is an option to refuse to take any input from\nothers on this list on parts of what you have already done, if you\nwant to be in my tree, anywhere, not even in contrib/.\n\nJust like we say \"the plumbing command X has behaved this way\nforever, and we cannot break the backward compatibility\" to some\npossible changes to any of our tools, however, \"Scalar has been used\nwith that subcommand behaving this particular way even before the\ncode was donated to Git project, so we cannot lightly change it\"\nwould equally be a good input when we evaluate when some things in\nit may need to be changed, so I do not think it would pose a great\nbackward compatibility issue (after all, we do not change things\nlightly just for the sake of changing it).\n\nBut approaching our developers with a new piece of code, saying \"I\nwant to piggyback on your popularity. Among the stuff given to you,\nsome are out of limit and you may not even discuss about modifying\nit\" upfront, is a problematic attitude.\n\nNow, the goal from purely Git project's point of view would be to\nimprove Git's user experience, not necessarily Scalar's, by exposing\nmore of our users to the way its opinionated (which is *not*\nnecessarily a bad thing) decisions work to help its users, and our\ndevelopers to the scalar codebase. Some of our developers may not\neven agree with the design decisions Scalar made and that is\nexpected and is fine; based on such a reaction, the lesson Git\nlearns from Scalar to solve a similar issue may turn out to be solve\nit differently.\n\nI do not know how much Options 1-3 would help such cross pollination\nmore over the Option 0, offhand, though.\n\n\n> Possible approaches to land in our final target\n> -----------------------------------------------\n\nHere I sense the \"final target\" is somehow Scalar is delivered to\nend-users through my tree and being maintained there, but from my\npoint of view, that is merely an early step. Wasn't the final\ntarget for \"git\" to learn from Scalar's successes and mistakes and\ngive native support to itches Scalar wanted to scratch?\n\nAnd if that is indeed the goal, when it is achieved, Scalar would\nbecome a historical curiosity just like Cogito is today. With\nCogito, we cross-pollinated just fine without having both in the\nsame tree at all, and when Cogito outlived its usefulness, we did\nnot have to remove anything from our tree. With cgit, we did not\ncross-pollinate at all, unfortunately, and gitweb more or less stays\nthe same way as it was years ago, and cgit is still a project with a\nuseful product. We can reach both extremes even if we take Option\n0. But it would have been messy for something like Cogito, which\nwas an early UI and workflow experiment, if we took a more\nintegrated approach.\n\n> **Phase 1.** Keep code in contrib/ while we contribute the Scalar features.\n>\n> Since the current patches on the mailing list are not feature complete, it\n> can be beneficial to move forward with the code in `contrib/scalar/` until\n> we reach feature parity. At that point, we could move the source into its\n> final resting place.\n>\n> This leans into the fact that `contrib/` contains \"interesting tools...\n> maybe even experimental ones\". While Scalar is not feature complete, it\n> can be isolated as experimental here until it is ready for promotion to\n> non-experimental.\n\nI personally have no problem with this approach, but with less\nscrutiny, being in contrib/ longer, with \"some stuff are not subject\nto discussion\" attitude, would inevitably lower the code quality of\nthe product, and it is dubious it would become \"ready for promotion\"\never. The contrib/ hierarchy, especially in early days when we\nwanted a way to grow the ecosystem, as a place to host \"these are\ndone externally, and we only carry them for convenience\", was a\nsuccessful model, but I do not think of anything that successfully\nused contrib/ as a nursery to later graduate to the core and to be\nseen as the same quality as other things that started their life in\ncore. I am pessimistic to the model that uses contrib/ as such a\nplace.\n\n> Depending on the final goal, we could drop some of the work that is currently\n> built within the `contrib/scalar/` directory, such as `-c`/`-C` parsing or\n> documentation builds. These features would be reimplemented in the new\n> location, so we can prevent that duplicate effort if we have a different\n> final location in mind.\n\n> **Phase 2.** Establish community standards on optional components\n>\n> While the work in `contrib/scalar/` continues to reach feature parity, we\n> can work as a community to set some ground rules about these kinds of\n> optional features (depending on how \"optional\" we land). This can include\n> standards such as how the files are laid out in the repository and how\n> they interact with the `Makefile`. Whatever we do for Scalar is likely to\n> form an example for a future contribution that we can't predict today.\n> Many of the questions we are asking today can be written for posterity:\n>\n> * What is an appropriate level of coupling with the Git codebase?\n> * Where should code custom to the component live?\n> * How should documentation and tests be organized?\n> * How do we initialize builds of a component?\n> * Who is responsible for supporting the component? Who fixes the bugs?\n> * Who is responsible for reviewing changes to the component?\n\nI am afraid that the answer to the first four are \"it depends\" (but\nthe remaining two would fall out naturally out of them).\n\nThanks. My brain is fried with the -rc work, rereading the material\nthat went to -rc0 since the last release, so I may have more to add\nlater, but for now, the above is what came to my mind.\n\n\n"},{"id":"440446","messageId":"nycvar.QRO.7.76.6.2111031833580.56@tvgsbejvaqbjf.bet","threadId":"56797","inReplyTo":"xmqqbl33a2fr.fsf@gitster.g","subject":"Re: [Discussion] The architecture of Scalar (and others) within Git","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-11-04T10:25:34Z","receivedAt":"2021-11-04T10:25:56Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Mon, 1 Nov 2021, Junio C Hamano wrote:\n\n> Derrick Stolee writes:\n>\n> > Optimization Factors\n> > --------------------\n> >\n> > There are multiple factors involved that we wish to optimize. Here is\n> > my understanding of the most-important factors and how they interact:\n> >\n> > ### Value\n> >\n> > The main reason to include Scalar within the Git codebase is to make\n> > it available to more users. This should have downstream effects based\n> > on user feedback about what they like or dislike about the Scalar CLI\n> > that can inform future features in core Git that simplify repository\n> > management at scale.\n>\n> In other words, Scalar wants to piggyback on Git's popularity, yet wants\n> to retain total control of how its end-user experience should look like,\n> eating the cake and having it at the same time?\n>\n> I do not think it is fair to proceed with this discussion without having\n> \"Scalar lives in its own tree, uses this mailing list to discuss how it\n> should evolve, drawing expertise from the contributors found here, but\n> otherwise would stay to be a spearate project\" as another option and\n> debating its pros-and-cons, next to the spectrum among contrib/scalar,\n> scalar/, and scalar.c approaches.\n\nIf that was indeed the intention with Scalar, I would totally agree.\n\nWe don't want Scalar to piggy-back onto Git. In fact, we don't even want\nit to be separate from Git, which is why we are contributing it here. I\nfirmly believe that Scalar has a lot of value and will benefit the Git\nproject and community. That is my motivation behind contributing these\npatches.\n\n> IIUC, cgit has one-directional coupling to depend on libgit.a just like\n> the Scalar wants to link with it---I am not saying Scalar should do the\n> same, but just raising it as an example that you do *not* have to live\n> in my tree anywhere to be able to link with libgit.a; that would\n> probably count as an \"**Option 0:** A separate project that borrows from\n> Git by having it as a submodule\".\n\nI did consider keeping Scalar/C in its own repository, and I, too, had\ncgit in mind (and cinnabar). In fact, Scalar/C does live in its own\nrepository right now, and would remain there if the community doesn’t want\nit: in the microsoft/git fork of git/git.\n\nHowever, just like partial clone, scheduled maintenance, sparse-checkout\ncone mode and sparse-index, the remainder of Scalar can benefit Git by\nmoving its functionality to `libgit.a`, or reimplementing it there.\n\n> I can think of only one obvious advantage going that route, and that is\n> why I said \"I am not saying Scalar should do the same\", which is that\n> the Scalar project will then retain total control over how it is built\n> and how its end-user experience should stay the same.\n>\n> I do not think it is an option to refuse to take any input from others\n> on this list on parts of what you have already done, if you want to be\n> in my tree, anywhere, not even in contrib/.\n>\n> Just like we say \"the plumbing command X has behaved this way forever,\n> and we cannot break the backward compatibility\" to some possible changes\n> to any of our tools, however, \"Scalar has been used with that subcommand\n> behaving this particular way even before the code was donated to Git\n> project, so we cannot lightly change it\" would equally be a good input\n> when we evaluate when some things in it may need to be changed, so I do\n> not think it would pose a great backward compatibility issue (after all,\n> we do not change things lightly just for the sake of changing it).\n\nThat's a really good point.\n\n> But approaching our developers with a new piece of code, saying \"I want\n> to piggyback on your popularity. Among the stuff given to you, some are\n> out of limit and you may not even discuss about modifying it\" upfront,\n> is a problematic attitude.\n\nYes, I agree, that would be a problematic attitude. I apologize for giving\nyou the impression that that was my attitude. I want to contribute Scalar\nto Git because I believe that Git will benefit from it, not because I want\nto abuse Git’s popularity to advance Scalar.\n\n> Now, the goal from purely Git project's point of view would be to\n> improve Git's user experience, not necessarily Scalar's, by exposing\n> more of our users to the way its opinionated (which is *not* necessarily\n> a bad thing) decisions work to help its users, and our developers to the\n> scalar codebase. Some of our developers may not even agree with the\n> design decisions Scalar made and that is expected and is fine; based on\n> such a reaction, the lesson Git learns from Scalar to solve a similar\n> issue may turn out to be solve it differently.\n>\n> I do not know how much Options 1-3 would help such cross pollination\n> more over the Option 0, offhand, though.\n\nI am still of the opinion that having the code in the same tree will make\ncross pollination much easier.\n\n> > Possible approaches to land in our final target\n> > -----------------------------------------------\n>\n> Here I sense the \"final target\" is somehow Scalar is delivered to\n> end-users through my tree and being maintained there, but from my point\n> of view, that is merely an early step. Wasn't the final target for\n> \"git\" to learn from Scalar's successes and mistakes and give native\n> support to itches Scalar wanted to scratch?\n\nI think we are very much in alignment here, and I believe Stolee's goal\nwas to reach such an alignment.\n\n> And if that is indeed the goal, when it is achieved, Scalar would become\n> a historical curiosity just like Cogito is today. With Cogito, we\n> cross-pollinated just fine without having both in the same tree at all,\n> and when Cogito outlived its usefulness, we did not have to remove\n> anything from our tree. With cgit, we did not cross-pollinate at all,\n> unfortunately, and gitweb more or less stays the same way as it was\n> years ago, and cgit is still a project with a useful product. We can\n> reach both extremes even if we take Option 0. But it would have been\n> messy for something like Cogito, which was an early UI and workflow\n> experiment, if we took a more integrated approach.\n\nScalar is more similar to cgit than to gitweb or Cogito in this context:\ngitweb and Cogito are scripted users of Git, whereas Scalar and cgit link\nto libgit.a, are written in C, and it is easy to refactor code from Scalar\ninto libgit.a. That was never the case with gitweb nor with Cogito.\n\n> > **Phase 1.** Keep code in contrib/ while we contribute the Scalar\n> > features.\n> >\n> > Since the current patches on the mailing list are not feature\n> > complete, it can be beneficial to move forward with the code in\n> > `contrib/scalar/` until we reach feature parity. At that point, we\n> > could move the source into its final resting place.\n> >\n> > This leans into the fact that `contrib/` contains \"interesting\n> > tools... maybe even experimental ones\". While Scalar is not feature\n> > complete, it can be isolated as experimental here until it is ready\n> > for promotion to non-experimental.\n>\n> I personally have no problem with this approach, but with less scrutiny,\n> being in contrib/ longer, with \"some stuff are not subject to\n> discussion\" attitude, would inevitably lower the code quality of the\n> product, and it is dubious it would become \"ready for promotion\" ever.\n\nTo be clear, I'm not pushing for promotion of Scalar to a top-level\ncommand delivered via the Git project at all. I _might_ push for it in the\nfuture, _iff_ a wider audience has used Scalar _and_ finds it useful.\n\nIn any case, pushing for Scalar to be such a top-level command would be\npremature at this stage: to avoid overwhelming reviewers, the Scalar patch\nseries is not yet feature-complete, there are other patch series I intend\nto contribute on top, when the first patch series has settled down.\n\n> The contrib/ hierarchy, especially in early days when we wanted a way to\n> grow the ecosystem, as a place to host \"these are done externally, and\n> we only carry them for convenience\", was a successful model, but I do\n> not think of anything that successfully used contrib/ as a nursery to\n> later graduate to the core and to be seen as the same quality as other\n> things that started their life in core. I am pessimistic to the model\n> that uses contrib/ as such a place.\n\nAfter seeing `vcs-svn`'s fate, I understand the trepidation. I, too, would\nhave loved to see it being cooked to perfection and then graduate.\n\nEven so, I targeted `contrib/` with my patch series because\n`contrib/README` says:\n\n The intention is to keep interesting tools around git here, maybe\n even experimental ones, to give users an easier access to them,\n and to give tools wider exposure, so that they can be improved\n faster.\n\nDoes this README's intent still apply, or has the purpose of contrib/\nchanged?\n\n> > Depending on the final goal, we could drop some of the work that is\n> > currently built within the `contrib/scalar/` directory, such as\n> > `-c`/`-C` parsing or documentation builds. These features would be\n> > reimplemented in the new location, so we can prevent that duplicate\n> > effort if we have a different final location in mind.\n>\n> > **Phase 2.** Establish community standards on optional components\n> >\n> > While the work in `contrib/scalar/` continues to reach feature parity,\n> > we can work as a community to set some ground rules about these kinds\n> > of optional features (depending on how \"optional\" we land). This can\n> > include standards such as how the files are laid out in the repository\n> > and how they interact with the `Makefile`. Whatever we do for Scalar\n> > is likely to form an example for a future contribution that we can't\n> > predict today. Many of the questions we are asking today can be\n> > written for posterity:\n> >\n> > * What is an appropriate level of coupling with the Git codebase?\n> > * Where should code custom to the component live?\n> > * How should documentation and tests be organized?\n> > * How do we initialize builds of a component?\n> > * Who is responsible for supporting the component? Who fixes the bugs?\n> > * Who is responsible for reviewing changes to the component?\n>\n> I am afraid that the answer to the first four are \"it depends\" (but the\n> remaining two would fall out naturally out of them).\n\nWould you like me to organize the code and patches such that they more\nclearly fall under your maintenance once the patches are merged? If that\nis your preference, I will gladly put in the work.\n\nWhen I submitted the Scalar patches, I did not want to burden you with\nmaintaining it, I expected to maintain Scalar going forward, but maybe\nthat is not your preference? I assumed it to be my maintenance burden, we\naim to support backward compatibility for existing Scalar users, after\nall. Absolutely everything else, however, is up for discussion, subject to\nthe full scrutiny of the community and to the same standards of a\nsubmission to the core of the project.\n\nAnd I see your point about backward compatibility: The Git project has\nplenty of experience with maintaining backward compatibility and _still_\nimproving on initial designs. I therefore can easily give up my hard “this\ncommand-line interface must not be changed” stance.\n\n> Thanks. My brain is fried with the -rc work, rereading the material\n> that went to -rc0 since the last release, so I may have more to add\n> later, but for now, the above is what came to my mind.\n\nThank you for taking the time to respond, especially during the -rc cycle.\nI look forward to continuing this thread when you have time.\n\nCiao,\nDscho\n\n\n"},{"id":"440470","messageId":"9eb8fd45-c8a5-0320-6d38-56389bef193d@gmail.com","threadId":"56797","inReplyTo":"211029.86k0hww317.gmgdl@evledraar.gmail.com","subject":"Re: [Discussion] The architecture of Scalar (and others) within Git","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2021-11-04T17:20:15Z","receivedAt":"2021-11-04T17:20:22Z","isPatch":false,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 10/28/2021 2:58 PM, Ævar Arnfjörð Bjarmason wrote:\n> \n> On Wed, Oct 27 2021, Derrick Stolee wrote:\n> \n>> I'm starting this discussion thread to create a new area to consider these\n>> high-level concepts. Specifically: how should a new component like Scalar\n>> be included in the Git codebase?\n\nThank you for your thoughts on this topic. I wanted to specifically\nreply to a few items while the discussion continues to simmer.\n\n> E.g. the scalar series is now at v6, but apparently it hasn't been\n> noticed that some of the tests were broken on OSX due to apparent errors\n> with launchctl integration. That's because the patches had the code\n> compiled, but didn't run the tests.\n\nThese failures are due to changes to Git maintenance during the 2.34\ncycle, and we've noticed and fixed them in microsoft/git as we rebased\non the release candidates.\n\nThis is one of the things that we expect with one extreme side of Git's\n\"ownership\" of Scalar: things could break Scalar that we won't notice\nuntil manually running the tests or taking upstream changes into our\nfork, but then we will fix them. Right now, that fix would be in a\nfuture iteration of the patch series.\n\n>> [...]\n>> **Option 1:** One-directional coupling in `contrib/scalar/`\n>>\n>> This option was our initial choice because it minimizes the responsibility\n>> of the Git community. While `contrib/scalar/scalar.c` depends on code in\n>> `libgit.a`, the implementation does not require that code to change to\n>> accommodate the needs of Scalar. The test suite and documentation is\n>> separate.\n>>\n>> This does mean that changes to `libgit.a` could break Scalar without any\n>> feedback to the developer that has not compiled Scalar. The Scalar\n>> maintainers would then need to watch for this and send separate updates to\n>> Scalar that fix these dependency breaks. This reduces developer friction,\n>> but might cause some extra burden on the Git maintainer. If these \"catch\n>> up to dependency breaks\" updates happen only after a release candidate is\n>> out, then maybe this isn't too much of a burden. We don't typically have\n>> release candidates for minor releases, so there is some risk there that\n>> minor releases could include breaks.\n>>\n>> If we make our CI builds include Scalar by default, then the previous\n>> paragraph about developer friction is negated.\n> \n> Without going into exhaustive detail I'll just note as above that much\n> of this describes a state that's got little or nothing to do with the\n> contrib/scalar/* patches on-list, which are thoroughly\n> bi-bidirectionally integrated, build (but don't test) by default etc.\n\nYou're right that I am describing a theoretical extreme that is not\nrealized by the patch series in its current form. The extreme I'm\ndescribing is where we could revert Scalar out of the Git codebase with\na simple 'rm -f contrib/scalar' and as minimal remaining work as possible.\nJohannes relaxed that condition in an attempt to reduce duplication in the\nMakefiles, but there is opportunity to redesign and get much closer to\nthis extreme, if that's the consensus we reach.\n\nThanks,\n-Stolee\n"},{"id":"440501","messageId":"xmqq7ddnz115.fsf@gitster.g","threadId":"56797","inReplyTo":"nycvar.QRO.7.76.6.2111031833580.56@tvgsbejvaqbjf.bet","subject":"Re: [Discussion] The architecture of Scalar (and others) within Git","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-11-05T04:20:22Z","receivedAt":"2021-11-05T04:20:26Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> Even so, I targeted `contrib/` with my patch series because\n> `contrib/README` says:\n>\n> The intention is to keep interesting tools around git here, maybe\n> even experimental ones, to give users an easier access to them,\n> and to give tools wider exposure, so that they can be improved\n> faster.\n>\n> Does this README's intent still apply, or has the purpose of contrib/\n> changed?\n\nThe intent may still be the same, but my suspicion is that the world\nhas changed sufficiently to make contrib/ that is offered with such\nan intent no longer is useful. In other words, \"contrib/\" as a\nnursery may have been our dream, but the dream did not materialize\nand may have outlived its usefulness.\n\n> Would you like me to organize the code and patches such that they more\n> clearly fall under your maintenance once the patches are merged? If that\n> is your preference, I will gladly put in the work.\n>\n> When I submitted the Scalar patches, I did not want to burden you with\n> maintaining it, I expected to maintain Scalar going forward, but maybe\n> that is not your preference? I assumed it to be my maintenance burden, we\n> aim to support backward compatibility for existing Scalar users, after\n> all. Absolutely everything else, however, is up for discussion, subject to\n> the full scrutiny of the community and to the same standards of a\n> submission to the core of the project.\n>\n> And I see your point about backward compatibility: The Git project has\n> plenty of experience with maintaining backward compatibility and _still_\n> improving on initial designs. I therefore can easily give up my hard “this\n> command-line interface must not be changed” stance.\n\nIt depends on how you define \"maintenance\".\n\nMany parts of the system (e.g. difftool, commit-graph, pack bitmaps,\nmulti-pack index, sparse-index, po, and gitweb) may \"fall under my\nmaintenance\", but you can see that I am not heavily involved in any\nof these areas, other than serving mostly as a traffic cop. If a\npatch that touches some area comes, I may trust input from folks who\nare more deeply involved in the area (e.g. input from Taylor better\nthan from other folks, including mine, for a patch that touches\nmidx) when deciding if the patch needs revising or if it is ready.\n\nI would expect that is more or less how things would work for\nanything that is in my tree. And I do not think it would be any\ndifferent for Scalar. I would expect that any bugfixes or new\nfeatures would be redirected to you folks. Ehh, not that you would\nneed redirecting---I'd expect you'd jump on them even _before_ I\nwould react ;-)\n"},{"id":"457767","messageId":"patch-v2-1.1-9743e2a1e6a-20220623T100554Z-avarab@gmail.com","threadId":"56797","inReplyTo":"cover-v2-0.1-00000000000-20220623T100554Z-avarab@gmail.com","subject":"[PATCH v2 1/1] scalar: reorganize from contrib/, still keep it \"a contrib command\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-06-23T10:26:23Z","receivedAt":"2022-06-23T10:26:42Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change the optional scalar command added in 0a43fb22026 (scalar:\ncreate a rudimentary executable, 2021-12-03) so that it builds, tests\nand CI's with our usual build and testing process, but is still kept\n\"a contrib command\" in how we talk about it in our documentation.\n\nBefore this change we've been building scalar.o in CI as part of the\n\"static-analysis\" job, we'll now build the full binary, and run its\ntests. We won't install it unless \"INSTALL_SCALAR=Y\" is provided to\nthe Makefile. Before this change there was no \"make install\" step for\nthe \"scalar\" binary at all.\n\nBefore this we needed a significant amount of Makefile boilerplate to\ndo something that the top-level Makefile could do with relatively\nlittle work, and by doing so we'll get the advantage of testing and\nCI-ing the scalar command along with our other commands.\n\nThe recent [1] shows a (now retracted, see [2]) series to integrate\nscalar with CI while it still lived in contrib/*. For context: There's\nbeen past discussion about how this command should be integrated in\nterms of the \"closeness\" of its integration, and how \"contrib\" sources\nwithin git.git should be organized.\n\nThat discussion started with my [3], was followed by a WIP patch[4] to\nimplement the approach being finished up here, which prompted the\n\"[Discussion] The architecture of Scalar (and others) within Git\"\nthread[4]. See also the large earlier discussion hanging off [4]. An\nearlier version of this change was proposed at [6].\n\nPart of the reason for sticking the new command in \"contrib/scalar/\"\nwas to implicitly make it clear from the paths that it was\ndifferent. I do think that would be a worthwhile goal in the abstract.\n\nBut given the build simplifications we can attain by moving it to the\ntop level that we should seek to resolve that ambiguity in the minds\nof any potential git.git code maintainers in some other way.\n\nTo do that we now have a new 'OPTIONAL CONTRIB COMMANDS' section in\n\"git help git\" explaining our backwards and forwards compatibility\nnon-promises when it comes to this and similar future commands. Our\nusers are the ones most likely to be confused about why their git\npackage has installed a \"/usr/bin/scalar\" that seemingly duplicates\nparts of git's own functionality. We're much better off by\ncross-linking our documentation, and mentioning the state of scalar in\ngit's own documentation, along with the full list of porcelain and\nplumbing utilities.\n\nCommentary on changes in specific sub-components:\n\nA.: The tests now live in \"t/t7990-scalar.sh\" (per the naming guide in\n t/README).\n\n Previously it had to be manually tested by cd-ing to\n contrib/scalar and running \"make test\", and it would not benefit\n from the combination of our various CI targets.\n\nB.: The \"Documentation/cmd-list.perl\" change is needed because that\n command wasn't capable of understanding commands in the\n command-list.txt whose name didn't start with \"git\".\n\n If we don't make this change we won't cross-link to \"scalar(1)\"\n from \"OPTIONAL CONTRIB COMMANDS\" in \"git(1)\".\n\nC. We'll always mention scalar in \"OPTIONAL CONTRIB COMMANDS\" in\n \"git(1)\", but the \"scalar(1)\" manpage itself depends on whether we\n build with \"INSTALL_SCALAR=Y\".\n\nD.: We'll unconditionally build scalar's documentation, and it will be\n linted along with the rest, including checking for broken links to\n other documentation.\n\n The minor change to scalar.txt here is needed due to the lint\n check added in cafd9828e89 (doc lint: lint and fix missing \"GIT\"\n end sections, 2021-04-09), perhaps we should have a different end\n blurb for this command, but for now let's make it consistent with\n the rest.\n\n When installed (see below) this command is part of the git suite,\n so the end blurb should say something to that effect.\n\nE.: \"make install\" will now install the \"scalar\" binary, but only if\n \"INSTALL_SCALAR\" is defined.\n\n Relative to our $(prefix) we'll now have (this is with\n INSTALL_SYMLINKS=Y):\n\n libexec/git-core/scalar: symbolic link to ../../bin/scalar\n bin/scalar: ELF 64-bit LSB pie executable,\n\n Putting it into libexec isn't strictly necessary, but as we do it\n with \"git\" we do that by default, and this will ensure that anyone\n who relies on their path being \"$(git --exec-path)\" will also be\n able to find \"scalar\".\n\n Perhaps we shouldn't put it in \"libexec\" at all, but for now let's\n just follow the herd.\n\nF.: Likewise \"make install-man install-doc install-html\" works, and\n will install \"scalar\" documentation if \"INSTALL_SCALAR\" is\n defined.\n\n We'll install these files with those targets (and correctly split\n them by target, e.g. only scalar.1 with \"install-man\"):\n\n share/doc/git-doc/scalar.txt: ASCII text\n share/doc/git-doc/scalar.html: XML 1.0 document, ASCII text, with CRLF line terminators\n share/man/man1/scalar.1: troff or preprocessor input, ASCII text, with very long lines\n\n== Fixed ==\n\nG.: The dependency graph of contrib/scalar/Makefile was broken when it\n came to scalar.o, it only depended on ../../libgit.a. So it was a\n requirement to run the top-level Makefile first to get around some\n of its dependency issues:\n\n make && make -C contrib/scalar\n\nH.: By having the top-level Makefile build scalar.o it'll work as well\n as any other git code, including benefiting from\n COMPUTE_HEADER_DEPENDENCIES.\n\n Targets such as (and possibly others):\n\n - \"make tags TAGS cscope\"\n - \"make coccicheck\"\n\t- \"make check-docs\"\n\n Wouldn't consider contrib/scalar at all, some of those would have\n been fixable, but as shown in the rest of this patch it's easier\n just to have scalar built like any other program instead.\n\n1. https://lore.kernel.org/git/220602.86ee07z6qb.gmgdl@evledraar.gmail.com/\n2. https://lore.kernel.org/git/nycvar.QRO.7.76.6.2206132246010.353@tvgsbejvaqbjf.bet/\n3. https://lore.kernel.org/git/87czpuxbwp.fsf@evledraar.gmail.com/\n4. https://lore.kernel.org/git/87ilz44kdk.fsf@evledraar.gmail.com/\n5. https://lore.kernel.org/git/b67bbef4-e4c3-b6a7-1c7f-7d405902ef8b@gmail.com/\n6: https://lore.kernel.org/git/patch-1.1-86fb8d56307-20211028T185016Z-avarab@gmail.com/\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n .gitignore | 1 +\n Documentation/Makefile | 4 +\n Documentation/cmd-list.perl | 2 +-\n Documentation/git.txt | 12 +++\n {contrib/scalar => Documentation}/scalar.txt | 4 +-\n Makefile | 57 +++++++++----\n command-list.txt | 2 +\n contrib/buildsystems/CMakeLists.txt | 15 +++-\n contrib/scalar/.gitignore | 2 -\n contrib/scalar/Makefile | 35 --------\n contrib/scalar/t/Makefile | 81 -------------------\n contrib/scalar/scalar.c => scalar.c | 0\n .../t/t9099-scalar.sh => t/t7990-scalar.sh | 8 +-\n 13 files changed, 79 insertions(+), 144 deletions(-)\n rename {contrib/scalar => Documentation}/scalar.txt (99%)\n delete mode 100644 contrib/scalar/.gitignore\n delete mode 100644 contrib/scalar/Makefile\n delete mode 100644 contrib/scalar/t/Makefile\n rename contrib/scalar/scalar.c => scalar.c (100%)\n rename contrib/scalar/t/t9099-scalar.sh => t/t7990-scalar.sh (96%)\n\ndiff --git a/.gitignore b/.gitignore\nindex a4522157641..b06d0923ade 100644\n--- a/.gitignore\n+++ b/.gitignore\n@@ -219,6 +219,7 @@\n /configure\n /.vscode/\n /tags\n+/scalar\n /TAGS\n /cscope*\n /compile_commands.json\ndiff --git a/Documentation/Makefile b/Documentation/Makefile\nindex f2e7fc1daa5..10fff5976af 100644\n--- a/Documentation/Makefile\n+++ b/Documentation/Makefile\n@@ -21,6 +21,9 @@ MAN1_TXT += $(filter-out \\\n MAN1_TXT += git.txt\n MAN1_TXT += gitk.txt\n MAN1_TXT += gitweb.txt\n+ifndef NO_INSTALL_SCALAR_DOC\n+MAN1_TXT += scalar.txt\n+endif\n \n # man5 / man7 guides (note: new guides should also be added to command-list.txt)\n MAN5_TXT += gitattributes.txt\n@@ -284,6 +287,7 @@ endif\n cmds_txt = cmds-ancillaryinterrogators.txt \\\n \tcmds-ancillarymanipulators.txt \\\n \tcmds-mainporcelain.txt \\\n+\tcmds-optionalcontrib.txt \\\n \tcmds-plumbinginterrogators.txt \\\n \tcmds-plumbingmanipulators.txt \\\n \tcmds-synchingrepositories.txt \\\ndiff --git a/Documentation/cmd-list.perl b/Documentation/cmd-list.perl\nindex af5da45d287..0f4b1b23cfe 100755\n--- a/Documentation/cmd-list.perl\n+++ b/Documentation/cmd-list.perl\n@@ -10,7 +10,7 @@ sub format_one {\n \t$state = 0;\n \topen I, '<', \"$name.txt\" or die \"No such file $name.txt\";\n \twhile () {\n-\t\tif (/^git[a-z0-9-]*\\(([0-9])\\)$/) {\n+\t\tif (/^[a-z0-9-]*\\(([0-9])\\)$/) {\n \t\t\t$mansection = $1;\n \t\t\tnext;\n \t\t}\ndiff --git a/Documentation/git.txt b/Documentation/git.txt\nindex 302607a4967..10c1979396b 100644\n--- a/Documentation/git.txt\n+++ b/Documentation/git.txt\n@@ -339,6 +339,18 @@ The following documentation pages are guides about Git concepts.\n \n include::cmds-guide.txt[]\n \n+Optional contrib commands\n+-------------------------\n+\n+The following commands are included with the git sources, but may not\n+be present in your installation.\n+\n+These should be considered \"contrib\"-level when it comes to\n+maintenance and stability promises. They might not even be included in\n+your installation, and may either drastically change in the future, or\n+go away entirely.\n+\n+include::cmds-optionalcontrib.txt[]\n \n Configuration Mechanism\n -----------------------\ndiff --git a/contrib/scalar/scalar.txt b/Documentation/scalar.txt\nsimilarity index 99%\nrename from contrib/scalar/scalar.txt\nrename to Documentation/scalar.txt\nindex c0425e06533..b11e9eecf21 100644\n--- a/contrib/scalar/scalar.txt\n+++ b/Documentation/scalar.txt\n@@ -162,6 +162,6 @@ SEE ALSO\n --------\n linkgit:git-clone[1], linkgit:git-maintenance[1].\n \n-Scalar\n+GIT\n ---\n-Associated with the linkgit:git[1] suite\n+Part of the linkgit:git[1] suite\ndiff --git a/Makefile b/Makefile\nindex 04d0fd1fe60..28294cd3626 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -469,6 +469,11 @@ include shared.mak\n # INSTALL_STRIP can be set to \"-s\" to strip binaries during installation,\n # if your $(INSTALL) command supports the option.\n #\n+# Define INSTALL_SCALAR if you would like to install the optional\n+# \"scalar\" command. This command is considered \"contrib\"-level, see\n+# 'Optional \"contrib\" commands' in the built (with \"make man\") git(1)\n+# manual page.\n+#\n # Define GENERATE_COMPILATION_DATABASE to \"yes\" to generate JSON compilation\n # database entries during compilation if your compiler supports it, using the\n # `-MJ` flag. The JSON entries will be placed in the `compile_commands/`\n@@ -602,6 +607,7 @@ GIT_OBJS =\n LIB_OBJS =\n OBJECTS =\n PROGRAM_OBJS =\n+OTHER_PROGRAMS =\n PROGRAMS =\n EXCLUDED_PROGRAMS =\n SCRIPT_PERL =\n@@ -812,7 +818,8 @@ BUILT_INS += git-switch$X\n BUILT_INS += git-whatchanged$X\n \n # what 'all' will build but not install in gitexecdir\n-OTHER_PROGRAMS = git$X\n+OTHER_PROGRAMS += git$X\n+OTHER_PROGRAMS += scalar$X\n \n # what test wrappers are needed and 'install' will install, in bindir\n BINDIR_PROGRAMS_NEED_X += git\n@@ -821,6 +828,18 @@ BINDIR_PROGRAMS_NEED_X += git-shell\n BINDIR_PROGRAMS_NEED_X += git-upload-archive\n BINDIR_PROGRAMS_NEED_X += git-upload-pack\n \n+# Sometimes we only have a test wrapper, but not a program to\n+# install. This isn't so pretty, and we could refactor the\n+# bin-wrappers/% and install code to make it unnecessary.\n+ifdef INSTALL_SCALAR\n+PROGRAMS += scalar$X\n+BINDIR_PROGRAMS_NEED_X += scalar\n+endif\n+TEST_BINDIR_PROGRAMS_NEED_X = $(BINDIR_PROGRAMS_NEED_X)\n+ifndef INSTALL_SCALAR\n+TEST_BINDIR_PROGRAMS_NEED_X += scalar\n+endif\n+\n BINDIR_PROGRAMS_NO_X += git-cvsserver\n \n # Set paths to tools early so that they can be used for version tests.\n@@ -2543,7 +2562,7 @@ ifndef NO_CURL\n \tOBJECTS += http.o http-walker.o remote-curl.o\n endif\n \n-SCALAR_SOURCES := contrib/scalar/scalar.c\n+SCALAR_SOURCES := scalar.c\n SCALAR_OBJECTS := $(SCALAR_SOURCES:c=o)\n OBJECTS += $(SCALAR_OBJECTS)\n \n@@ -2678,7 +2697,7 @@ $(REMOTE_CURL_PRIMARY): remote-curl.o http.o http-walker.o GIT-LDFLAGS $(GITLIBS\n \t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \\\n \t\t$(CURL_LIBCURL) $(EXPAT_LIBEXPAT) $(LIBS)\n \n-contrib/scalar/scalar$X: $(SCALAR_OBJECTS) GIT-LDFLAGS $(GITLIBS)\n+scalar$X: $(SCALAR_OBJECTS) GIT-LDFLAGS $(GITLIBS)\n \t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) \\\n \t\t$(filter %.o,$^) $(LIBS)\n \n@@ -3041,15 +3060,18 @@ GIT-PYTHON-VARS: FORCE\n fi\n endif\n \n-test_bindir_programs := $(patsubst %,bin-wrappers/%,$(BINDIR_PROGRAMS_NEED_X) $(BINDIR_PROGRAMS_NO_X) $(TEST_PROGRAMS_NEED_X))\n+test_bindir_programs := $(patsubst %,bin-wrappers/%,$(TEST_BINDIR_PROGRAMS_NEED_X) $(BINDIR_PROGRAMS_NO_X) $(TEST_PROGRAMS_NEED_X))\n \n all:: $(TEST_PROGRAMS) $(test_bindir_programs)\n \n+# Substitution that's only done on programs named git-* in\n+# bin-wrappers/*\n+GIT_TEST_BINDIR_PROGRAMS_NEED_X = $(filter-out scalar,$(TEST_BINDIR_PROGRAMS_NEED_X))\n bin-wrappers/%: wrap-for-bin.sh\n \t$(call mkdir_p_parent_template)\n \t$(QUIET_GEN)sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \\\n \t -e 's|@@BUILD_DIR@@|$(shell pwd)|' \\\n-\t -e 's|@@PROG@@|$(patsubst test-%,t/helper/test-%$(X),$(@F))$(patsubst git%,$(X),$(filter $(@F),$(BINDIR_PROGRAMS_NEED_X)))|' < $< > $@ && \\\n+\t -e 's|@@PROG@@|$(patsubst test-%,t/helper/test-%$(X),$(@F))$(patsubst git%,$(X),$(filter $(@F),$(GIT_TEST_BINDIR_PROGRAMS_NEED_X)))|' < $< > $@ && \\\n \tchmod +x $@\n \n # GNU make supports exporting all variables by \"export\" without parameters.\n@@ -3281,11 +3303,18 @@ endif\n install-gitweb:\n \t$(MAKE) -C gitweb install\n \n+# We must not \"export\" this as e.g. \"check-docs\" needs to know about\n+# scalar.txt. We only exclude scalar.txt for the \"install-*\" targets.\n+NO_INSTALL_SCALAR_DOC =\n+ifndef INSTALL_SCALAR\n+NO_INSTALL_SCALAR_DOC = NoScalarPlease\n+endif\n+\n install-doc: install-man-perl\n-\t$(MAKE) -C Documentation install\n+\t$(MAKE) -C Documentation install NO_INSTALL_SCALAR_DOC='$(NO_INSTALL_SCALAR_DOC)'\n \n install-man: install-man-perl\n-\t$(MAKE) -C Documentation install-man\n+\t$(MAKE) -C Documentation install-man NO_INSTALL_SCALAR_DOC='$(NO_INSTALL_SCALAR_DOC)'\n \n install-man-perl: man-perl\n \t$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(mandir_SQ)/man3'\n@@ -3293,22 +3322,22 @@ install-man-perl: man-perl\n \t(cd '$(DESTDIR_SQ)$(mandir_SQ)/man3' && umask 022 && $(TAR) xof -)\n \n install-html:\n-\t$(MAKE) -C Documentation install-html\n+\t$(MAKE) -C Documentation install-html NO_INSTALL_SCALAR_DOC='$(NO_INSTALL_SCALAR_DOC)'\n \n install-info:\n-\t$(MAKE) -C Documentation install-info\n+\t$(MAKE) -C Documentation install-info NO_INSTALL_SCALAR_DOC='$(NO_INSTALL_SCALAR_DOC)'\n \n install-pdf:\n-\t$(MAKE) -C Documentation install-pdf\n+\t$(MAKE) -C Documentation install-pdf NO_INSTALL_SCALAR_DOC='$(NO_INSTALL_SCALAR_DOC)'\n \n quick-install-doc:\n-\t$(MAKE) -C Documentation quick-install\n+\t$(MAKE) -C Documentation quick-install NO_INSTALL_SCALAR_DOC='$(NO_INSTALL_SCALAR_DOC)'\n \n quick-install-man:\n-\t$(MAKE) -C Documentation quick-install-man\n+\t$(MAKE) -C Documentation quick-install-man NO_INSTALL_SCALAR_DOC='$(NO_INSTALL_SCALAR_DOC)'\n \n quick-install-html:\n-\t$(MAKE) -C Documentation quick-install-html\n+\t$(MAKE) -C Documentation quick-install-html NO_INSTALL_SCALAR_DOC='$(NO_INSTALL_SCALAR_DOC)'\n \n \n \n@@ -3497,7 +3526,7 @@ check-docs::\n \t\t -e 's/\\.txt//'; \\\n \t) | while read how cmd; \\\n \tdo \\\n-\t\tcase \" $(patsubst %$X,%,$(ALL_COMMANDS) $(BUILT_INS) $(EXCLUDED_PROGRAMS)) \" in \\\n+\t\tcase \" $(patsubst %$X,%,$(ALL_COMMANDS) $(BUILT_INS) $(EXCLUDED_PROGRAMS) scalar) \" in \\\n \t\t*\" $$cmd \"*)\t;; \\\n \t\t*) echo \"removed but $$how: $$cmd\" ;; \\\n \t\tesac; \\\ndiff --git a/command-list.txt b/command-list.txt\nindex 9bd6f3c48f4..721f968b3d1 100644\n--- a/command-list.txt\n+++ b/command-list.txt\n@@ -16,6 +16,7 @@\n # synchingrepositories\n # synchelpers\n # purehelpers\n+# optionalcontrib\n #\n # The type names are self explanatory. But if you want to see what\n # command belongs to what group to get a better picture, have a look\n@@ -215,3 +216,4 @@ gittutorial guide\n gittutorial-2 guide\n gitweb ancillaryinterrogators\n gitworkflows guide\n+scalar optionalcontrib\ndiff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt\nindex 1b23f2440d8..3de83f7f454 100644\n--- a/contrib/buildsystems/CMakeLists.txt\n+++ b/contrib/buildsystems/CMakeLists.txt\n@@ -610,7 +610,8 @@ unset(CMAKE_REQUIRED_INCLUDES)\n #programs\n set(PROGRAMS_BUILT\n \tgit git-daemon git-http-backend git-sh-i18n--envsubst\n-\tgit-shell)\n+\tgit-shell\n+\tscalar)\n \n if(NOT CURL_FOUND)\n \tlist(APPEND excluded_progs git-http-fetch git-http-push)\n@@ -745,6 +746,9 @@ list(TRANSFORM git_SOURCES PREPEND \"${CMAKE_SOURCE_DIR}/\")\n add_executable(git ${CMAKE_SOURCE_DIR}/git.c ${git_SOURCES})\n target_link_libraries(git common-main)\n \n+add_executable(scalar ${CMAKE_SOURCE_DIR}/scalar.c)\n+target_link_libraries(scalar common-main)\n+\n add_executable(git-daemon ${CMAKE_SOURCE_DIR}/daemon.c)\n target_link_libraries(git-daemon common-main)\n \n@@ -902,10 +906,16 @@ list(TRANSFORM git_shell_scripts PREPEND \"${CMAKE_BINARY_DIR}/\")\n list(TRANSFORM git_perl_scripts PREPEND \"${CMAKE_BINARY_DIR}/\")\n \n #install\n+option(INSTALL_SCALAR \"Install the optional 'scalar' contrib command\")\n foreach(program ${PROGRAMS_BUILT})\n if(program STREQUAL \"git\" OR program STREQUAL \"git-shell\")\n install(TARGETS ${program}\n \tRUNTIME DESTINATION bin)\n+elseif(program STREQUAL \"scalar\")\n+if(INSTALL_SCALAR)\n+install(TARGETS ${program}\n+\tRUNTIME DESTINATION bin)\n+endif()\n else()\n install(TARGETS ${program}\n \tRUNTIME DESTINATION libexec/git-core)\n@@ -977,7 +987,8 @@ endif()\n \n #wrapper scripts\n set(wrapper_scripts\n-\tgit git-upload-pack git-receive-pack git-upload-archive git-shell git-remote-ext)\n+\tgit git-upload-pack git-receive-pack git-upload-archive git-shell git-remote-ext\n+\tscalar)\n \n set(wrapper_test_scripts\n \ttest-fake-ssh test-tool)\ndiff --git a/contrib/scalar/.gitignore b/contrib/scalar/.gitignore\ndeleted file mode 100644\nindex ff3d47e84d0..00000000000\n--- a/contrib/scalar/.gitignore\n+++ /dev/null\n@@ -1,2 +0,0 @@\n-/*.exe\n-/scalar\ndiff --git a/contrib/scalar/Makefile b/contrib/scalar/Makefile\ndeleted file mode 100644\nindex 37f283f35d7..00000000000\n--- a/contrib/scalar/Makefile\n+++ /dev/null\n@@ -1,35 +0,0 @@\n-# The default target of this Makefile is...\n-all::\n-\n-# Import tree-wide shared Makefile behavior and libraries\n-include ../../shared.mak\n-\n-include ../../config.mak.uname\n--include ../../config.mak.autogen\n--include ../../config.mak\n-\n-TARGETS = scalar$(X) scalar.o\n-GITLIBS = ../../common-main.o ../../libgit.a ../../xdiff/lib.a\n-\n-all:: scalar$(X) ../../bin-wrappers/scalar\n-\n-$(GITLIBS):\n-\t$(QUIET_SUBDIR0)../.. $(QUIET_SUBDIR1) $(subst ../../,,$@)\n-\n-$(TARGETS): $(GITLIBS) scalar.c\n-\t$(QUIET_SUBDIR0)../.. $(QUIET_SUBDIR1) $(patsubst %,contrib/scalar/%,$@)\n-\n-clean:\n-\t$(RM) $(TARGETS) ../../bin-wrappers/scalar\n-\n-../../bin-wrappers/scalar: ../../wrap-for-bin.sh Makefile\n-\t@mkdir -p ../../bin-wrappers\n-\t$(QUIET_GEN)sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \\\n-\t -e 's|@@BUILD_DIR@@|$(shell cd ../.. && pwd)|' \\\n-\t -e 's|@@PROG@@|contrib/scalar/scalar$(X)|' < $< > $@ && \\\n-\tchmod +x $@\n-\n-test: all\n-\t$(MAKE) -C t\n-\n-.PHONY: $(GITLIBS) all clean test FORCE\ndiff --git a/contrib/scalar/t/Makefile b/contrib/scalar/t/Makefile\ndeleted file mode 100644\nindex 01e82e56d15..00000000000\n--- a/contrib/scalar/t/Makefile\n+++ /dev/null\n@@ -1,81 +0,0 @@\n-# Import tree-wide shared Makefile behavior and libraries\n-include ../../../shared.mak\n-\n-# Run scalar tests\n-#\n-# Copyright (c) 2005,2021 Junio C Hamano, Johannes Schindelin\n-#\n-\n--include ../../../config.mak.autogen\n--include ../../../config.mak\n-\n-SHELL_PATH ?= $(SHELL)\n-PERL_PATH ?= /usr/bin/perl\n-RM ?= rm -f\n-PROVE ?= prove\n-DEFAULT_TEST_TARGET ?= test\n-TEST_LINT ?= test-lint\n-\n-ifdef TEST_OUTPUT_DIRECTORY\n-TEST_RESULTS_DIRECTORY = $(TEST_OUTPUT_DIRECTORY)/test-results\n-else\n-TEST_RESULTS_DIRECTORY = ../../../t/test-results\n-endif\n-\n-# Shell quote;\n-SHELL_PATH_SQ = $(subst ','\\'',$(SHELL_PATH))\n-PERL_PATH_SQ = $(subst ','\\'',$(PERL_PATH))\n-TEST_RESULTS_DIRECTORY_SQ = $(subst ','\\'',$(TEST_RESULTS_DIRECTORY))\n-\n-T = $(sort $(wildcard t[0-9][0-9][0-9][0-9]-*.sh))\n-\n-all: $(DEFAULT_TEST_TARGET)\n-\n-test: $(TEST_LINT)\n-\t$(MAKE) aggregate-results-and-cleanup\n-\n-prove: $(TEST_LINT)\n-\t@echo \"*** prove ***\"; GIT_CONFIG=.git/config $(PROVE) --exec '$(SHELL_PATH_SQ)' $(GIT_PROVE_OPTS) $(T) :: $(GIT_TEST_OPTS)\n-\t$(MAKE) clean-except-prove-cache\n-\n-$(T):\n-\t@echo \"*** $@ ***\"; GIT_CONFIG=.git/config '$(SHELL_PATH_SQ)' $@ $(GIT_TEST_OPTS)\n-\n-clean-except-prove-cache:\n-\t$(RM) -r 'trash directory'.* '$(TEST_RESULTS_DIRECTORY_SQ)'\n-\t$(RM) -r valgrind/bin\n-\n-clean: clean-except-prove-cache\n-\t$(RM) .prove\n-\n-test-lint: test-lint-duplicates test-lint-executable test-lint-shell-syntax\n-\n-test-lint-duplicates:\n-\t@dups=`echo $(T) | tr ' ' '\\n' | sed 's/-.*//' | sort | uniq -d` && \\\n-\t\ttest -z \"$$dups\" || { \\\n-\t\techo >&2 \"duplicate test numbers:\" $$dups; exit 1; }\n-\n-test-lint-executable:\n-\t@bad=`for i in $(T); do test -x \"$$i\" || echo $$i; done` && \\\n-\t\ttest -z \"$$bad\" || { \\\n-\t\techo >&2 \"non-executable tests:\" $$bad; exit 1; }\n-\n-test-lint-shell-syntax:\n-\t@'$(PERL_PATH_SQ)' ../../../t/check-non-portable-shell.pl $(T)\n-\n-aggregate-results-and-cleanup: $(T)\n-\t$(MAKE) aggregate-results\n-\t$(MAKE) clean\n-\n-aggregate-results:\n-\tfor f in '$(TEST_RESULTS_DIRECTORY_SQ)'/t*-*.counts; do \\\n-\t\techo \"$$f\"; \\\n-\tdone | '$(SHELL_PATH_SQ)' ../../../t/aggregate-results.sh\n-\n-valgrind:\n-\t$(MAKE) GIT_TEST_OPTS=\"$(GIT_TEST_OPTS) --valgrind\"\n-\n-test-results:\n-\tmkdir -p test-results\n-\n-.PHONY: $(T) aggregate-results clean valgrind\ndiff --git a/contrib/scalar/scalar.c b/scalar.c\nsimilarity index 100%\nrename from contrib/scalar/scalar.c\nrename to scalar.c\ndiff --git a/contrib/scalar/t/t9099-scalar.sh b/t/t7990-scalar.sh\nsimilarity index 96%\nrename from contrib/scalar/t/t9099-scalar.sh\nrename to t/t7990-scalar.sh\nindex 10b1172a8aa..897c8e760ef 100755\n--- a/contrib/scalar/t/t9099-scalar.sh\n+++ b/t/t7990-scalar.sh\n@@ -2,13 +2,7 @@\n \n test_description='test the `scalar` command'\n \n-TEST_DIRECTORY=$PWD/../../../t\n-export TEST_DIRECTORY\n-\n-# Make it work with --no-bin-wrappers\n-PATH=$PWD/..:$PATH\n-\n-. ../../../t/test-lib.sh\n+. ./test-lib.sh\n \n GIT_TEST_MAINT_SCHEDULER=\"crontab:test-tool crontab ../cron.txt,launchctl:true,schtasks:true\"\n export GIT_TEST_MAINT_SCHEDULER\n-- \n2.36.1.1239.gfba91521d90\n\n"},{"id":"457768","messageId":"cover-v2-0.1-00000000000-20220623T100554Z-avarab@gmail.com","threadId":"56797","inReplyTo":"patch-1.1-86fb8d56307-20211028T185016Z-avarab@gmail.com","subject":"[PATCH v2 0/1] scalar: move to the top-level, test, CI and \"install\" support","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-06-23T10:26:22Z","receivedAt":"2022-06-23T10:26:45Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"This one-patch series integrates the \"scalar\" command to the\ntop-level, meaning we build the \"scalar\" binary by default, and run\nits tests on \"make test\" and in CI. We'll also build and test its\ndocumentation. We now also have \"install\" support, both for the\nprogram and its docs, but you'll need to:\n\n make INSTALL_SCALAR=Y\n\nI'm sending this out now to avoid needless duplicate work. A recent\nseries attempted to integrate scalar into the CI[1], but it's now been\nretracted[2]. As I'd discovered when I submitted the v1 of this[3]\nintegrating something like scalar \"halfway\" is much harder to do (see\ne.g. the problems with [1] that I pointed out in [4]).\n\nI then saw that Victoria Dye pushed out a WIP topic [5] with a patch\nthat's basically a subset of this one, but doesn't handle various\ntough edge cases handled here (and this one fully passes CI\n[6]). namely:\n\n * This adjusts the the \"cmake\" build, so VS build passes\n * This integrates with the documentation build, and targets like\n \"check-docs\"\n * This implements the optional \"make install\" support (much of which\n is predicated on getting the Documentation/ integration right)\n * This adds \"scalar\" to the command-list.txt, which has various\n positive downstream effects.\n\nAs in the v1 [3] this change proposed to create an \"optionalcontrib\"\ncategory in the command-list.txt. The \"scalar\" command is put into\nthat category, and in \"man git\" we say:\n\n\tOptional contrib commands\n\t-------------------------\n\n\tThe following commands are included with the git sources, but may not\n\tbe present in your installation.\n\n\tThese should be considered \"contrib\"-level when it comes to\n\tmaintenance and stability promises. They might not even be included in\n\tyour installation, and may either drastically change in the future, or\n\tgo away entirely.\n\n\tinclude::cmds-optionalcontrib.txt[]\n\nSo I think this change allows us to have our cake and eat it too as\nfar as the \"contrib status\" of a program like \"scalar\" is\nconcerned. I.e. we're merely changing the internal source organization\nhere to avoid duplicate work during git.git maintenance, but it will\nbe made clear to users that this isn't a normal command, and it's not\ninstalled by default.\n\nI think it would make sense as a follow-up to consider moving\ncontrib/{diff-highlight,mw-to-git,subtree} to a similar\nstructure. I.e. they also carry copy/pasted \"Makefile\" infrastructure\nin various ways, resulting in buggy integration with the rest\n(e.g. try to run \"make coverage\" in a way that includes \"git\nsubtree\").\n\nIf what we want is to e.g. not install those by default, or even not\nto run their tests or have them in CI by default as this change (and\nprevious discussion on the scalar topic) shows that's much easier than\nbridging the other gaps.\n\n1. https://lore.kernel.org/git/pull.1129.git.1654160735.gitgitgadget@gmail.com/\n2. https://lore.kernel.org/git/nycvar.QRO.7.76.6.2206132246010.353@tvgsbejvaqbjf.bet/\n3. https://lore.kernel.org/git/patch-1.1-86fb8d56307-20211028T185016Z-avarab@gmail.com/\n4. https://lore.kernel.org/git/220602.86ee07z6qb.gmgdl@evledraar.gmail.com/\n5. https://github.com/vdye/git/tree/feature/scalar-toplevel\n6. https://github.com/avar/git/actions/runs/2541684143\n\nÆvar Arnfjörð Bjarmason (1):\n scalar: reorganize from contrib/, still keep it \"a contrib command\"\n\n .gitignore | 1 +\n Documentation/Makefile | 4 +\n Documentation/cmd-list.perl | 2 +-\n Documentation/git.txt | 12 +++\n {contrib/scalar => Documentation}/scalar.txt | 4 +-\n Makefile | 57 +++++++++----\n command-list.txt | 2 +\n contrib/buildsystems/CMakeLists.txt | 15 +++-\n contrib/scalar/.gitignore | 2 -\n contrib/scalar/Makefile | 35 --------\n contrib/scalar/t/Makefile | 81 -------------------\n contrib/scalar/scalar.c => scalar.c | 0\n .../t/t9099-scalar.sh => t/t7990-scalar.sh | 8 +-\n 13 files changed, 79 insertions(+), 144 deletions(-)\n rename {contrib/scalar => Documentation}/scalar.txt (99%)\n delete mode 100644 contrib/scalar/.gitignore\n delete mode 100644 contrib/scalar/Makefile\n delete mode 100644 contrib/scalar/t/Makefile\n rename contrib/scalar/scalar.c => scalar.c (100%)\n rename contrib/scalar/t/t9099-scalar.sh => t/t7990-scalar.sh (96%)\n\nRange-diff against v1:\n1: 86fb8d56307 ! 1: 9743e2a1e6a contrib: build & test scalar by default, optionally install\n @@ Metadata\n Author: Ævar Arnfjörð Bjarmason \n \n ## Commit message ##\n - contrib: build & test scalar by default, optionally install\n -\n - Change how the newly proposed \"scalar\" command is handle, this builds\n - on top of the proposed patches that add it along with documentation\n - and tests to the \"contrib/scalar/\" directory[1].\n -\n - With those patches we unconditionally build scalar.o and link it to\n - libgit.a, but don't build the \"scalar\" binary itself, and don't run\n - its tests as part of our normal test suite run or CI.\n -\n - As the changes to \"t/t9099-scalar.sh\" here shows that sort of\n - arrangement leads to breakages. I.e. the scalar tests would fail on\n - OSX, which our OSX CI jobs will catch, but only if we run the\n - tests. Let's add a prerequisite to test that, which requires moving\n - around some of the setup code.\n -\n - I think that for a libgit.a-using \"scalar\" in particular, and for\n - \"contrib\" in general we're better off by not only compiling the object\n - in question, but also linking it, and running its tests by\n - default. What we won't do is install it by default, unless an\n - \"INSTALL_SCALAR=Y\" is provided along with \"make install\".\n -\n - For context: There's been ongoing discussion about how this command\n - should be integrated in terms of the \"closeness\" of its integration,\n - and how \"contrib\" sources within git.git should be organized.\n -\n - That discussion started with my [2], was followed by a WIP patch[3] to\n - implement the approach being finished up here, and has spawned the\n + scalar: reorganize from contrib/, still keep it \"a contrib command\"\n +\n + Change the optional scalar command added in 0a43fb22026 (scalar:\n + create a rudimentary executable, 2021-12-03) so that it builds, tests\n + and CI's with our usual build and testing process, but is still kept\n + \"a contrib command\" in how we talk about it in our documentation.\n +\n + Before this change we've been building scalar.o in CI as part of the\n + \"static-analysis\" job, we'll now build the full binary, and run its\n + tests. We won't install it unless \"INSTALL_SCALAR=Y\" is provided to\n + the Makefile. Before this change there was no \"make install\" step for\n + the \"scalar\" binary at all.\n +\n + Before this we needed a significant amount of Makefile boilerplate to\n + do something that the top-level Makefile could do with relatively\n + little work, and by doing so we'll get the advantage of testing and\n + CI-ing the scalar command along with our other commands.\n +\n + The recent [1] shows a (now retracted, see [2]) series to integrate\n + scalar with CI while it still lived in contrib/*. For context: There's\n + been past discussion about how this command should be integrated in\n + terms of the \"closeness\" of its integration, and how \"contrib\" sources\n + within git.git should be organized.\n +\n + That discussion started with my [3], was followed by a WIP patch[4] to\n + implement the approach being finished up here, which prompted the\n \"[Discussion] The architecture of Scalar (and others) within Git\"\n - thread[4]. See also the large earlier discussion hanging off [3].\n + thread[4]. See also the large earlier discussion hanging off [4]. An\n + earlier version of this change was proposed at [6].\n \n - I really don't think it's important whether a given source file lives\n - in-tree at \"contrib/scalar/scalar.c\" or the top-level \"scalar.c\", but\n - as the diff below shows the latter approach is a smaller overall\n - change in our build system, due to how it's organized, more details on\n - that below under \"Build changes\".\n -\n - I think that part of the reason for sticking the new command in\n - \"contrib/scalar/\" was to implicitly make it clear from the paths that\n - it was different. I do think that would be a worthwhile goal in the\n - abstract.\n + Part of the reason for sticking the new command in \"contrib/scalar/\"\n + was to implicitly make it clear from the paths that it was\n + different. I do think that would be a worthwhile goal in the abstract.\n \n But given the build simplifications we can attain by moving it to the\n top level that we should seek to resolve that ambiguity in the minds\n @@ Commit message\n git's own documentation, along with the full list of porcelain and\n plumbing utilities.\n \n - = Build changes =\n -\n - This fixes dependency bugs in the previous \"contrib/scalar/Makefile\", as\n - well as implementing various missing bits of functionality, most\n - notably \"make install\" will now install the \"scalar\" command, but only\n - if INSTALL_SCALAR is defined.\n -\n - Those and other changes and non-changes, categorized as \"Same\", \"New\"\n - and \"Fixed\" below:\n -\n - == Same ==\n -\n - A.: We'll unconditionally build scalar.o, as before it's in the\n - $(OBJECTS) list.\n -\n - B.: \"make contrib/scalar/scalar.o\" has the same dependency graph as\n - before, but is now spelled \"make scalar.o\", more on the rename below.\n + Commentary on changes in specific sub-components:\n \n - == New ==\n -\n - C.: We'll now unconditionally build and test the scalar command.\n - Before we'd only build scalar.o, but not link it.\n -\n - Its test lives in \"t/t9099-scalar.sh\" now (and take <1s to\n - run). We should have test coverage of this in-tree command that's\n - linking to libgit.a.\n + A.: The tests now live in \"t/t7990-scalar.sh\" (per the naming guide in\n + t/README).\n \n Previously it had to be manually tested by cd-ing to\n contrib/scalar and running \"make test\", and it would not benefit\n from the combination of our various CI targets.\n \n + B.: The \"Documentation/cmd-list.perl\" change is needed because that\n + command wasn't capable of understanding commands in the\n + command-list.txt whose name didn't start with \"git\".\n +\n + If we don't make this change we won't cross-link to \"scalar(1)\"\n + from \"OPTIONAL CONTRIB COMMANDS\" in \"git(1)\".\n +\n + C. We'll always mention scalar in \"OPTIONAL CONTRIB COMMANDS\" in\n + \"git(1)\", but the \"scalar(1)\" manpage itself depends on whether we\n + build with \"INSTALL_SCALAR=Y\".\n +\n D.: We'll unconditionally build scalar's documentation, and it will be\n linted along with the rest, including checking for broken links to\n other documentation.\n @@ Commit message\n been fixable, but as shown in the rest of this patch it's easier\n just to have scalar built like any other program instead.\n \n - == Discussion and motivation ==\n -\n - I've been fixing various dependency issues in git's Makefile\n - infrastructure recently, some of which has been integrated into\n - \"master\", some of which is currently under review on-list, and some of\n - which isn't submitted yet.\n -\n - To the extent that we have build dependency issues and cases where we\n - can't build something in parallel it's usually because we're invoking\n - one Makefile from another. That's the case in most of our\n - sub-Makefiles, which will usually need to invoke or depend on\n - something created in the top-level Makefile.\n -\n - None of {t,Documentation,template}/Makefile etc. are truly\n - independent, as looking at $(git grep -F '../' '*Makefile') will\n - reveal, and we'll sometimes need to duplicate logic between them\n - purely to get around them not being driven by one top-level Makefile\n - logic.\n -\n - Much has been written on this topic, likely most notably the\n - \"Recursive Make Considered Harmful\" paper (full PDF link at [5]), and\n - to be fair, the there's also a well-known \"Non-recursive Make\n - Considered Harmful\" retort at [6]).\n -\n - Theoretical opinions on the topic clearly differ. From a purely\n - practical perspective I ran into textual and semantic conflicts with\n - the \"contrib/scalar/\" work.\n -\n - I think the diffstat of this patch demonstrates that this is a much\n - simpler approach, particularly considering that the code being\n - replaced didn't perform much of the needed integrations that are \"new\"\n - here. E.g. we now have \"make install\". The unsubmitted [7] (linked to\n - by [8]) shows the boilerplate we'd need for that with the alternate\n - approach.\n -\n - It would be more pleasing to not have to hardcode \"git\" in the\n - pre-image and now \"git\" and \"scalar\" in the post-image in several\n - places. My initial WIP [3] looks better, but built on a set of\n - Makefile reorganization patches.\n -\n - I'd like to not make those patches a dependency of this change, but\n - readers should rest assured that we're really not ending up with as\n - many \"scalar\" special-cases in the long term as this diff indicates,\n - most or all of those can all be generalized into Makefile refactoring\n - that teaches our build system to build N number of differently named\n - top-level commands.\n -\n - 1. https://lore.kernel.org/git/pull.1005.v6.git.1635323239.gitgitgadget@gmail.com/\n - 2. https://lore.kernel.org/git/87czpuxbwp.fsf@evledraar.gmail.com/\n - 3. https://lore.kernel.org/git/87ilz44kdk.fsf@evledraar.gmail.com/\n - 4. https://lore.kernel.org/git/b67bbef4-e4c3-b6a7-1c7f-7d405902ef8b@gmail.com/\n - 5. https://web.archive.org/web/20150330111905/http://miller.emu.id.au/pmiller/books/rmch/\n - 6. https://www.microsoft.com/en-us/research/wp-content/uploads/2016/03/hadrian.pdf\n - 7. https://github.com/dscho/git/commit/473ca8ae673\n - 8. https://lore.kernel.org/git/nycvar.QRO.7.76.6.2110210947590.56@tvgsbejvaqbjf.bet/\n + 1. https://lore.kernel.org/git/220602.86ee07z6qb.gmgdl@evledraar.gmail.com/\n + 2. https://lore.kernel.org/git/nycvar.QRO.7.76.6.2206132246010.353@tvgsbejvaqbjf.bet/\n + 3. https://lore.kernel.org/git/87czpuxbwp.fsf@evledraar.gmail.com/\n + 4. https://lore.kernel.org/git/87ilz44kdk.fsf@evledraar.gmail.com/\n + 5. https://lore.kernel.org/git/b67bbef4-e4c3-b6a7-1c7f-7d405902ef8b@gmail.com/\n + 6: https://lore.kernel.org/git/patch-1.1-86fb8d56307-20211028T185016Z-avarab@gmail.com/\n \n Signed-off-by: Ævar Arnfjörð Bjarmason \n \n @@ Documentation/scalar.txt: SEE ALSO\n +Part of the linkgit:git[1] suite\n \n ## Makefile ##\n -@@ Makefile: all::\n +@@ Makefile: include shared.mak\n # INSTALL_STRIP can be set to \"-s\" to strip binaries during installation,\n # if your $(INSTALL) command supports the option.\n #\n @@ Makefile: GIT_OBJS =\n +OTHER_PROGRAMS =\n PROGRAMS =\n EXCLUDED_PROGRAMS =\n -+SCALAR_OBJS =\n SCRIPT_PERL =\n - SCRIPT_PYTHON =\n - SCRIPT_SH =\n @@ Makefile: BUILT_INS += git-switch$X\n BUILT_INS += git-whatchanged$X\n \n @@ Makefile: BINDIR_PROGRAMS_NEED_X += git-shell\n BINDIR_PROGRAMS_NO_X += git-cvsserver\n \n # Set paths to tools early so that they can be used for version tests.\n -@@ Makefile: git.sp git.s git.o: EXTRA_CPPFLAGS = \\\n - \t'-DGIT_MAN_PATH=\"$(mandir_relative_SQ)\"' \\\n - \t'-DGIT_INFO_PATH=\"$(infodir_relative_SQ)\"'\n - \n --git$X: git.o GIT-LDFLAGS $(BUILTIN_OBJS) $(GITLIBS)\n --\t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) \\\n --\t\t$(filter %.o,$^) $(LIBS)\n -+define top-level-cmd-rule\n -+$(1)$X: $(1).o GIT-LDFLAGS $$(GITLIBS) $(2)\n -+\t$$(QUIET_LINK)$$(CC) $$(ALL_CFLAGS) -o $$@ $$(ALL_LDFLAGS) \\\n -+\t\t$$(filter %.o,$$^) $(LIBS)\n -+endef\n -+$(eval $(call top-level-cmd-rule,git,$(BUILTIN_OBJS)))\n -+$(eval $(call top-level-cmd-rule,scalar,))\n - \n - help.sp help.s help.o: command-list.h\n - hook.sp hook.s hook.o: hook-list.h\n -@@ Makefile: GIT_OBJS += git.o\n - .PHONY: git-objs\n - git-objs: $(GIT_OBJS)\n - \n -+SCALAR_OBJS += scalar.o\n -+.PHONY: scalar-objs\n -+scalar-objs: $(SCALAR_OBJS)\n -+\n - OBJECTS += $(GIT_OBJS)\n -+OBJECTS += $(SCALAR_OBJS)\n - OBJECTS += $(PROGRAM_OBJS)\n - OBJECTS += $(TEST_OBJS)\n - OBJECTS += $(XDIFF_OBJS)\n @@ Makefile: ifndef NO_CURL\n \tOBJECTS += http.o http-walker.o remote-curl.o\n endif\n \n -SCALAR_SOURCES := contrib/scalar/scalar.c\n --SCALAR_OBJECTS := $(SCALAR_SOURCES:c=o)\n --OBJECTS += $(SCALAR_OBJECTS)\n --\n - .PHONY: objects\n - objects: $(OBJECTS)\n ++SCALAR_SOURCES := scalar.c\n + SCALAR_OBJECTS := $(SCALAR_SOURCES:c=o)\n + OBJECTS += $(SCALAR_OBJECTS)\n \n @@ Makefile: $(REMOTE_CURL_PRIMARY): remote-curl.o http.o http-walker.o GIT-LDFLAGS $(GITLIBS\n \t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \\\n \t\t$(CURL_LIBCURL) $(EXPAT_LIBEXPAT) $(LIBS)\n \n -contrib/scalar/scalar$X: $(SCALAR_OBJECTS) GIT-LDFLAGS $(GITLIBS)\n --\t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) \\\n --\t\t$(filter %.o,$^) $(LIBS)\n --\n - $(LIB_FILE): $(LIB_OBJS)\n - \t$(QUIET_AR)$(RM) $@ && $(AR) $(ARFLAGS) $@ $^\n ++scalar$X: $(SCALAR_OBJECTS) GIT-LDFLAGS $(GITLIBS)\n + \t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) \\\n + \t\t$(filter %.o,$^) $(LIBS)\n \n @@ Makefile: GIT-PYTHON-VARS: FORCE\n fi\n @@ Makefile: GIT-PYTHON-VARS: FORCE\n +# bin-wrappers/*\n +GIT_TEST_BINDIR_PROGRAMS_NEED_X = $(filter-out scalar,$(TEST_BINDIR_PROGRAMS_NEED_X))\n bin-wrappers/%: wrap-for-bin.sh\n - \t@mkdir -p bin-wrappers\n + \t$(call mkdir_p_parent_template)\n \t$(QUIET_GEN)sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \\\n \t -e 's|@@BUILD_DIR@@|$(shell pwd)|' \\\n -\t -e 's|@@PROG@@|$(patsubst test-%,t/helper/test-%$(X),$(@F))$(patsubst git%,$(X),$(filter $(@F),$(BINDIR_PROGRAMS_NEED_X)))|' < $< > $@ && \\\n @@ Makefile: endif\n install-gitweb:\n \t$(MAKE) -C gitweb install\n \n --install-doc: install-man-perl\n --\t$(MAKE) -C Documentation install\n --\n --install-man: install-man-perl\n --\t$(MAKE) -C Documentation install-man\n -+ifdef INSTALL_SCALAR\n ++# We must not \"export\" this as e.g. \"check-docs\" needs to know about\n ++# scalar.txt. We only exclude scalar.txt for the \"install-*\" targets.\n +NO_INSTALL_SCALAR_DOC =\n -+else\n ++ifndef INSTALL_SCALAR\n +NO_INSTALL_SCALAR_DOC = NoScalarPlease\n +endif\n ++\n + install-doc: install-man-perl\n +-\t$(MAKE) -C Documentation install\n ++\t$(MAKE) -C Documentation install NO_INSTALL_SCALAR_DOC='$(NO_INSTALL_SCALAR_DOC)'\n + \n + install-man: install-man-perl\n +-\t$(MAKE) -C Documentation install-man\n ++\t$(MAKE) -C Documentation install-man NO_INSTALL_SCALAR_DOC='$(NO_INSTALL_SCALAR_DOC)'\n \n install-man-perl: man-perl\n \t$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(mandir_SQ)/man3'\n - \t(cd perl/build/man/man3 && $(TAR) cf - .) | \\\n +@@ Makefile: install-man-perl: man-perl\n \t(cd '$(DESTDIR_SQ)$(mandir_SQ)/man3' && umask 022 && $(TAR) xof -)\n \n --install-html:\n + install-html:\n -\t$(MAKE) -C Documentation install-html\n --\n --install-info:\n ++\t$(MAKE) -C Documentation install-html NO_INSTALL_SCALAR_DOC='$(NO_INSTALL_SCALAR_DOC)'\n + \n + install-info:\n -\t$(MAKE) -C Documentation install-info\n --\n --install-pdf:\n ++\t$(MAKE) -C Documentation install-info NO_INSTALL_SCALAR_DOC='$(NO_INSTALL_SCALAR_DOC)'\n + \n + install-pdf:\n -\t$(MAKE) -C Documentation install-pdf\n -+define install-doc-rule\n -+$(1): $(3)\n -+\t$$(MAKE) -C Documentation $(2) NO_INSTALL_SCALAR_DOC=$$(NO_INSTALL_SCALAR_DOC)\n -+endef\n -+$(eval $(call install-doc-rule,install-doc,install,install-man-perl))\n -+$(eval $(call install-doc-rule,install-man,install-man,install-man-perl))\n -+$(eval $(call install-doc-rule,install-html,install-html,))\n -+$(eval $(call install-doc-rule,install-info,install-info,))\n -+$(eval $(call install-doc-rule,install-pdf,install-pdf,))\n ++\t$(MAKE) -C Documentation install-pdf NO_INSTALL_SCALAR_DOC='$(NO_INSTALL_SCALAR_DOC)'\n \n quick-install-doc:\n - \t$(MAKE) -C Documentation quick-install\n -@@ Makefile: ifneq ($(INCLUDE_DLLS_IN_ARTIFACTS),)\n - OTHER_PROGRAMS += $(shell echo *.dll t/helper/*.dll)\n - endif\n +-\t$(MAKE) -C Documentation quick-install\n ++\t$(MAKE) -C Documentation quick-install NO_INSTALL_SCALAR_DOC='$(NO_INSTALL_SCALAR_DOC)'\n + \n + quick-install-man:\n +-\t$(MAKE) -C Documentation quick-install-man\n ++\t$(MAKE) -C Documentation quick-install-man NO_INSTALL_SCALAR_DOC='$(NO_INSTALL_SCALAR_DOC)'\n + \n + quick-install-html:\n +-\t$(MAKE) -C Documentation quick-install-html\n ++\t$(MAKE) -C Documentation quick-install-html NO_INSTALL_SCALAR_DOC='$(NO_INSTALL_SCALAR_DOC)'\n + \n + \n \n --artifacts-tar:: $(ALL_COMMANDS_TO_INSTALL) $(SCRIPT_LIB) $(OTHER_PROGRAMS) \\\n --\t\tGIT-BUILD-OPTIONS $(TEST_PROGRAMS) $(test_bindir_programs) \\\n -+# No scalar in \"cmake\" yet, and since 4c2c38e800f (ci: modification of\n -+# main.yml to use cmake for vs-build job, 2020-06-26) the \"vs-build\"\n -+# job has a hard dependency on it. Let's fail in the tests instead of\n -+# in the \"vs-build\" job itself.\n -+ifeq ($(wildcard scalar),scalar)\n -+ARCHIVE_OTHER_PROGRAMS = $(OTHER_PROGRAMS)\n -+ARCHIVE_TEST_BINDIR_PROGRAMS = $(test_bindir_programs)\n -+else\n -+ARCHIVE_OTHER_PROGRAMS = $(filter-out scalar%, $(OTHER_PROGRAMS))\n -+ARCHIVE_TEST_BINDIR_PROGRAMS = $(filter-out bin-wrappers/scalar,$(test_bindir_programs))\n -+endif\n -+artifacts-tar:: $(ALL_COMMANDS_TO_INSTALL) $(SCRIPT_LIB) $(ARCHIVE_OTHER_PROGRAMS) \\\n -+\t\tGIT-BUILD-OPTIONS $(TEST_PROGRAMS) $(ARCHIVE_TEST_BINDIR_PROGRAMS) \\\n - \t\t$(MOFILES)\n - \t$(QUIET_SUBDIR0)templates $(QUIET_SUBDIR1) \\\n - \t\tSHELL_PATH='$(SHELL_PATH_SQ)' PERL_PATH='$(PERL_PATH_SQ)'\n @@ Makefile: check-docs::\n \t\t -e 's/\\.txt//'; \\\n \t) | while read how cmd; \\\n @@ command-list.txt\n #\n # The type names are self explanatory. But if you want to see what\n # command belongs to what group to get a better picture, have a look\n -@@ command-list.txt: gitsubmodules guide\n +@@ command-list.txt: gittutorial guide\n gittutorial-2 guide\n - gittutorial guide\n + gitweb ancillaryinterrogators\n gitworkflows guide\n +scalar optionalcontrib\n \n + ## contrib/buildsystems/CMakeLists.txt ##\n +@@ contrib/buildsystems/CMakeLists.txt: unset(CMAKE_REQUIRED_INCLUDES)\n + #programs\n + set(PROGRAMS_BUILT\n + \tgit git-daemon git-http-backend git-sh-i18n--envsubst\n +-\tgit-shell)\n ++\tgit-shell\n ++\tscalar)\n + \n + if(NOT CURL_FOUND)\n + \tlist(APPEND excluded_progs git-http-fetch git-http-push)\n +@@ contrib/buildsystems/CMakeLists.txt: list(TRANSFORM git_SOURCES PREPEND \"${CMAKE_SOURCE_DIR}/\")\n + add_executable(git ${CMAKE_SOURCE_DIR}/git.c ${git_SOURCES})\n + target_link_libraries(git common-main)\n + \n ++add_executable(scalar ${CMAKE_SOURCE_DIR}/scalar.c)\n ++target_link_libraries(scalar common-main)\n ++\n + add_executable(git-daemon ${CMAKE_SOURCE_DIR}/daemon.c)\n + target_link_libraries(git-daemon common-main)\n + \n +@@ contrib/buildsystems/CMakeLists.txt: list(TRANSFORM git_shell_scripts PREPEND \"${CMAKE_BINARY_DIR}/\")\n + list(TRANSFORM git_perl_scripts PREPEND \"${CMAKE_BINARY_DIR}/\")\n + \n + #install\n ++option(INSTALL_SCALAR \"Install the optional 'scalar' contrib command\")\n + foreach(program ${PROGRAMS_BUILT})\n + if(program STREQUAL \"git\" OR program STREQUAL \"git-shell\")\n + install(TARGETS ${program}\n + \tRUNTIME DESTINATION bin)\n ++elseif(program STREQUAL \"scalar\")\n ++if(INSTALL_SCALAR)\n ++install(TARGETS ${program}\n ++\tRUNTIME DESTINATION bin)\n ++endif()\n + else()\n + install(TARGETS ${program}\n + \tRUNTIME DESTINATION libexec/git-core)\n +@@ contrib/buildsystems/CMakeLists.txt: endif()\n + \n + #wrapper scripts\n + set(wrapper_scripts\n +-\tgit git-upload-pack git-receive-pack git-upload-archive git-shell git-remote-ext)\n ++\tgit git-upload-pack git-receive-pack git-upload-archive git-shell git-remote-ext\n ++\tscalar)\n + \n + set(wrapper_test_scripts\n + \ttest-fake-ssh test-tool)\n +\n ## contrib/scalar/.gitignore (deleted) ##\n @@\n --/*.xml\n --/*.1\n --/*.html\n -/*.exe\n -/scalar\n \n ## contrib/scalar/Makefile (deleted) ##\n @@\n --QUIET_SUBDIR0 = +$(MAKE) -C # space to separate -C and subdir\n --QUIET_SUBDIR1 =\n --\n --ifneq ($(findstring s,$(MAKEFLAGS)),s)\n --ifndef V\n --\tQUIET_GEN = @echo ' ' GEN $@;\n --\tQUIET_SUBDIR0 = +@subdir=\n --\tQUIET_SUBDIR1 = ;$(NO_SUBDIR) echo ' ' SUBDIR $$subdir; \\\n --\t\t\t $(MAKE) $(PRINT_DIR) -C $$subdir\n --\tQUIET = @\n --else\n --\texport V\n --endif\n --endif\n +-# The default target of this Makefile is...\n +-all::\n -\n --all:\n +-# Import tree-wide shared Makefile behavior and libraries\n +-include ../../shared.mak\n -\n -include ../../config.mak.uname\n --include ../../config.mak.autogen\n @@ contrib/scalar/Makefile (deleted)\n -TARGETS = scalar$(X) scalar.o\n -GITLIBS = ../../common-main.o ../../libgit.a ../../xdiff/lib.a\n -\n --all: scalar$(X) ../../bin-wrappers/scalar\n +-all:: scalar$(X) ../../bin-wrappers/scalar\n -\n -$(GITLIBS):\n -\t$(QUIET_SUBDIR0)../.. $(QUIET_SUBDIR1) $(subst ../../,,$@)\n @@ contrib/scalar/Makefile (deleted)\n -\n -clean:\n -\t$(RM) $(TARGETS) ../../bin-wrappers/scalar\n --\t$(RM) scalar.1 scalar.html scalar.xml\n -\n -../../bin-wrappers/scalar: ../../wrap-for-bin.sh Makefile\n -\t@mkdir -p ../../bin-wrappers\n @@ contrib/scalar/Makefile (deleted)\n -test: all\n -\t$(MAKE) -C t\n -\n --docs: scalar.html scalar.1\n --\n --scalar.html: | scalar.1 # prevent them from trying to build `doc.dep` in parallel\n --\n --scalar.html scalar.1: scalar.txt\n --\t$(QUIET_SUBDIR0)../../Documentation$(QUIET_SUBDIR1) \\\n --\t\tMAN_TXT=../contrib/scalar/scalar.txt \\\n --\t\t../contrib/scalar/$@\n --\t$(QUIET)test scalar.1 != \"$@\" || mv ../../Documentation/$@ .\n --\n --.PHONY: $(GITLIBS) all clean docs test FORCE\n +-.PHONY: $(GITLIBS) all clean test FORCE\n \n ## contrib/scalar/t/Makefile (deleted) ##\n @@\n +-# Import tree-wide shared Makefile behavior and libraries\n +-include ../../../shared.mak\n +-\n -# Run scalar tests\n -#\n -# Copyright (c) 2005,2021 Junio C Hamano, Johannes Schindelin\n @@ contrib/scalar/t/Makefile (deleted)\n \n ## contrib/scalar/scalar.c => scalar.c ##\n \n - ## contrib/scalar/t/t9099-scalar.sh => t/t9099-scalar.sh ##\n + ## contrib/scalar/t/t9099-scalar.sh => t/t7990-scalar.sh ##\n @@\n \n test_description='test the `scalar` command'\n \n -TEST_DIRECTORY=$PWD/../../../t\n -export TEST_DIRECTORY\n -+. ./test-lib.sh\n - \n +-\n -# Make it work with --no-bin-wrappers\n -PATH=$PWD/..:$PATH\n -\n -. ../../../t/test-lib.sh\n -+if test_have_prereq WINDOWS && ! scalar list 2>/dev/null\n -+then\n -+\tskip_all='the contrib cmake process does not build \"scalar\" yet'\n -+\ttest_done\n -+fi\n ++. ./test-lib.sh\n \n - GIT_TEST_MAINT_SCHEDULER=\"crontab:test-tool crontab ../cron.txt\"\n + GIT_TEST_MAINT_SCHEDULER=\"crontab:test-tool crontab ../cron.txt,launchctl:true,schtasks:true\"\n export GIT_TEST_MAINT_SCHEDULER\n -@@ t/t9099-scalar.sh: test_expect_success 'scalar shows a usage' '\n - \ttest_expect_code 129 scalar -h\n - '\n - \n --test_expect_success 'scalar unregister' '\n -+test_expect_success 'setup' '\n -+\ttest_commit first &&\n -+\ttest_commit second &&\n -+\ttest_commit third &&\n -+\tgit switch -c parallel first\n -+'\n -+\n -+test_lazy_prereq SCALAR_REGISTER '\n -+\tgit init test/src &&\n -+\tscalar register test/src &&\n -+\tscalar list >expect &&\n -+\tscalar unregister test/src &&\n -+\t(cd test/src && echo \"$PWD\") >actual &&\n -+\ttest_cmp expect actual\n -+'\n -+\n -+test_expect_success SCALAR_REGISTER 'scalar unregister' '\n - \tgit init vanish/src &&\n - \tscalar register vanish/src &&\n - \tgit config --get --global --fixed-value \\\n -@@ t/t9099-scalar.sh: test_expect_success 'scalar unregister' '\n - \t! grep -F \"$(pwd)/vanish/src\" scalar.repos\n - '\n - \n --test_expect_success 'set up repository to clone' '\n --\ttest_commit first &&\n --\ttest_commit second &&\n --\ttest_commit third &&\n --\tgit switch -c parallel first &&\n -+test_expect_success SCALAR_REGISTER 'set up repository to clone' '\n - \tmkdir -p 1/2 &&\n - \ttest_commit 1/2/3 &&\n - \tgit config uploadPack.allowFilter true &&\n - \tgit config uploadPack.allowAnySHA1InWant true\n - '\n - \n --test_expect_success 'scalar clone' '\n -+test_expect_success SCALAR_REGISTER 'scalar clone' '\n - \tsecond=$(git rev-parse --verify second:second.t) &&\n - \tscalar clone \"file://$(pwd)\" cloned --single-branch &&\n - \t(\n -@@ t/t9099-scalar.sh: test_expect_success 'scalar clone' '\n - \t)\n - '\n - \n --test_expect_success 'scalar reconfigure' '\n -+test_expect_success SCALAR_REGISTER 'scalar reconfigure' '\n - \tgit init one/src &&\n - \tscalar register one &&\n - \tgit -C one/src config core.preloadIndex false &&\n -@@ t/t9099-scalar.sh: test_expect_success 'scalar delete without enlistment shows a usage' '\n - \ttest_expect_code 129 scalar delete\n - '\n - \n --test_expect_success 'scalar delete with enlistment' '\n -+test_expect_success SCALAR_REGISTER 'scalar delete with enlistment' '\n - \tscalar delete cloned &&\n - \ttest_path_is_missing cloned\n - '\n-- \n2.36.1.1239.gfba91521d90\n\n"},{"id":"457786","messageId":"2f3067e1-43fb-26b3-83c4-6ca0722149a0@github.com","threadId":"56797","inReplyTo":"cover-v2-0.1-00000000000-20220623T100554Z-avarab@gmail.com","subject":"Re: [PATCH v2 0/1] scalar: move to the top-level, test, CI and \"install\" support","fromName":"Derrick Stolee","fromEmail":"derrickstolee@github.com","sentAt":"2022-06-23T15:02:29Z","receivedAt":"2022-06-23T15:02:35Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 6/23/2022 6:26 AM, Ævar Arnfjörð Bjarmason wrote:\n> This one-patch series integrates the \"scalar\" command to the\n> top-level, meaning we build the \"scalar\" binary by default, and run\n> its tests on \"make test\" and in CI. We'll also build and test its\n> documentation. We now also have \"install\" support, both for the\n> program and its docs, but you'll need to:\n> \n> make INSTALL_SCALAR=Y\n> \n> I'm sending this out now to avoid needless duplicate work.\n\nAs mentioned on the list earlier, Victoria is taking over the\nremaining work to complete the Scalar project. Nothing has been\nsent to the list because we didn't want to cause a distraction\nfrom the release window.\n\nVictoria is taking time to incorporate your previous thoughts on\nhow Scalar is built and its location in the codebase and create\na complete narrative of how to get from our current state to that\npoint.\n\nTo that point, this thread is the duplicate work you're trying\nto avoid. Please instead wait for Victoria to present her plan in\nJuly instead of moving forward with this topic.\n\nThanks,\n-Stolee\n\n"},{"id":"457796","messageId":"220623.86k097js9k.gmgdl@evledraar.gmail.com","threadId":"56797","inReplyTo":"2f3067e1-43fb-26b3-83c4-6ca0722149a0@github.com","subject":"Re: [PATCH v2 0/1] scalar: move to the top-level, test, CI and \"install\" support","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-06-23T15:30:00Z","receivedAt":"2022-06-23T15:47:57Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Jun 23 2022, Derrick Stolee wrote:\n\n> On 6/23/2022 6:26 AM, Ævar Arnfjörð Bjarmason wrote:\n>> This one-patch series integrates the \"scalar\" command to the\n>> top-level, meaning we build the \"scalar\" binary by default, and run\n>> its tests on \"make test\" and in CI. We'll also build and test its\n>> documentation. We now also have \"install\" support, both for the\n>> program and its docs, but you'll need to:\n>> \n>> make INSTALL_SCALAR=Y\n>> \n>> I'm sending this out now to avoid needless duplicate work.\n>\n> As mentioned on the list earlier, Victoria is taking over the\n> remaining work to complete the Scalar project. Nothing has been\n> sent to the list because we didn't want to cause a distraction\n> from the release window.\n\nI was on the fence about sending this out, but given that the \"CI\"\nthread was going on until the start of that window, and wanting to save\nher the work of re-discovering the subtle issues with the integration\nI'd already fixed I thought it was better ot send it out.\n\n> Victoria is taking time to incorporate your previous thoughts on\n> how Scalar is built and its location in the codebase and create\n> a complete narrative of how to get from our current state to that\n> point.\n\nI wasn't sure she was even aware of it, and given that the WIP patch I\nsaw in my \"git fetch\" was pretty much a subset of the upthread v1 it\nseemed that there was needless duplicate work going on.\n\nIt seemed clear that that WIP patch was attempting to head in the same\ndirection, but hadn't yet discovered some of the hurdles with\ne.g. documentation building & installation that I'd fixed\nalready. There's also the CMake integration, which I finished up for\nthis v2.\n\n> To that point, this thread is the duplicate work you're trying\n> to avoid. Please instead wait for Victoria to present her plan in\n> July instead of moving forward with this topic.\n\nBy \"duplicate work\" I mean that back in October of last year I sent a\npatch that was a more complete version of some WIP code I spotted\nwritten in the past few days.\n\nBut you're probably talking about duplication in the sense of some\nlarger integration of scalar changes into git.git? Or at least that's\nthe only way I can make sense of the seemingly reversed chronology.\n\nI'm all too happy to leave that to someone else that's more interested,\nperhaps this will save them some time.\n\nAlthough (sans release window etc.) I think this change is in a state\nthat's ready for pickup, and it seems that here's a consensus on this\ndirection from everyone involved.\n\nAlthough perhaps there's some minor disagreement about details, such as\nwhether we should have an \"optionalcontrib\" documentation section etc.\n\nI do think the current state on \"master\" is slightly confusing, one\noddity I spotted is that we've already asked translators to translate\ncontrib/scalar/scalar.c, but we never install it, and it's \"in\ncontrib\".\n\nIt seems that part was unintentional in 0a43fb22026 (scalar: create a\nrudimentary executable, 2021-12-03), and later formalized in 9f555783c0b\n(Makefile: generate \"po/git.pot\" from stable LOCALIZED_C,\n2022-05-26). Maybe it's all water under the bridge at this point,\ni.e. we'd rather keep it than throw away now-existing translations...\n\nFWIW I have this local change queued on top of this v2, it's all\ncosmetic, but probably a good idea.\n\nThe $(SCALAR_SOURCES) bit is something I missed, but which Victoria\ndidn't in her WIP patch (I stole it from there). That part had been\nadded since October, so I managed to miss it when rebasing, it's the\npart that's been adding contrib/scalar/scalar.c to po/git.pot (see\n9f555783c0b).\n\n1: 9743e2a1e6a ! 1: 11404988785 scalar: reorganize from contrib/, still keep it \"a contrib command\"\n @@ Commit message\n \n ## .gitignore ##\n @@\n + /config.mak.append\n /configure\n /.vscode/\n - /tags\n +/scalar\n + /tags\n /TAGS\n /cscope*\n - /compile_commands.json\n \n ## Documentation/Makefile ##\n @@ Documentation/Makefile: MAN1_TXT += $(filter-out \\\n @@ Makefile: ifndef NO_CURL\n endif\n \n -SCALAR_SOURCES := contrib/scalar/scalar.c\n -+SCALAR_SOURCES := scalar.c\n - SCALAR_OBJECTS := $(SCALAR_SOURCES:c=o)\n +-SCALAR_OBJECTS := $(SCALAR_SOURCES:c=o)\n ++SCALAR_OBJECTS := scalar.o\n OBJECTS += $(SCALAR_OBJECTS)\n \n + .PHONY: objects\n @@ Makefile: $(REMOTE_CURL_PRIMARY): remote-curl.o http.o http-walker.o GIT-LDFLAGS $(GITLIBS\n \t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \\\n \t\t$(CURL_LIBCURL) $(EXPAT_LIBEXPAT) $(LIBS)\n @@ Makefile: $(REMOTE_CURL_PRIMARY): remote-curl.o http.o http-walker.o GIT-LDFLAGS\n \t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) \\\n \t\t$(filter %.o,$^) $(LIBS)\n \n +@@ Makefile: XGETTEXT_FLAGS_SH = $(XGETTEXT_FLAGS) --language=Shell \\\n + XGETTEXT_FLAGS_PERL = $(XGETTEXT_FLAGS) --language=Perl \\\n + \t--keyword=__ --keyword=N__ --keyword=\"__n:1,2\"\n + MSGMERGE_FLAGS = --add-location --backup=off --update\n +-LOCALIZED_C = $(sort $(FOUND_C_SOURCES) $(FOUND_H_SOURCES) $(SCALAR_SOURCES) \\\n +-\t $(GENERATED_H))\n ++LOCALIZED_C = $(sort $(FOUND_C_SOURCES) $(FOUND_H_SOURCES) $(GENERATED_H))\n + LOCALIZED_SH = $(sort $(SCRIPT_SH) git-sh-setup.sh)\n + LOCALIZED_PERL = $(sort $(SCRIPT_PERL))\n + \n @@ Makefile: GIT-PYTHON-VARS: FORCE\n fi\n endif\n\n"},{"id":"457800","messageId":"c93c8e75-6c88-ac99-d8c3-1e2e7dd06ea3@github.com","threadId":"56797","inReplyTo":"220623.86k097js9k.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH v2 0/1] scalar: move to the top-level, test, CI and \"install\" support","fromName":"Derrick Stolee","fromEmail":"derrickstolee@github.com","sentAt":"2022-06-23T16:12:39Z","receivedAt":"2022-06-23T16:12:44Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 6/23/2022 11:30 AM, Ævar Arnfjörð Bjarmason wrote:\n> \n> On Thu, Jun 23 2022, Derrick Stolee wrote:\n> \n>> On 6/23/2022 6:26 AM, Ævar Arnfjörð Bjarmason wrote:\n>>> This one-patch series integrates the \"scalar\" command to the\n>>> top-level, meaning we build the \"scalar\" binary by default, and run\n>>> its tests on \"make test\" and in CI. We'll also build and test its\n>>> documentation. We now also have \"install\" support, both for the\n>>> program and its docs, but you'll need to:\n>>>\n>>> make INSTALL_SCALAR=Y\n>>>\n>>> I'm sending this out now to avoid needless duplicate work.\n>>\n>> As mentioned on the list earlier, Victoria is taking over the\n>> remaining work to complete the Scalar project. Nothing has been\n>> sent to the list because we didn't want to cause a distraction\n>> from the release window.\n> \n> I was on the fence about sending this out, but given that the \"CI\"\n> thread was going on until the start of that window, and wanting to save\n> her the work of re-discovering the subtle issues with the integration\n> I'd already fixed I thought it was better ot send it out.\n\nThe CI thread was halted not just because of the release window,\nbut because of a change in who was doing the work and taking time\nto revisit the plan of record. This was communicated on-list [1].\n\n[1] https://lore.kernel.org/git/nycvar.QRO.7.76.6.2206132246010.353@tvgsbejvaqbjf.bet/\n\nThe point is that the end goal is still \"Get Scalar out of contrib/\"\nbut the questions that are still being worked out are things like:\n\n1. _When_ do we integrate Scalar into CI?\n2. _When_ do we move Scalar out of contrib/?\n3. _When_ do we implement the remaining Scalar features?\n\nAll three of these questions depend on what the final result will\nlook like, and Victoria is still working on developing her own\nopinion on that before presenting it to the list in a complete\nform.\n\nYour patch here is interrupting that process.\n\n>> Victoria is taking time to incorporate your previous thoughts on\n>> how Scalar is built and its location in the codebase and create\n>> a complete narrative of how to get from our current state to that\n>> point.\n> \n> I wasn't sure she was even aware of it, and given that the WIP patch I\n> saw in my \"git fetch\" was pretty much a subset of the upthread v1 it\n> seemed that there was needless duplicate work going on.\n> \n> It seemed clear that that WIP patch was attempting to head in the same\n> direction, but hadn't yet discovered some of the hurdles with\n> e.g. documentation building & installation that I'd fixed\n> already. There's also the CMake integration, which I finished up for\n> this v2.\n\nPoking around in people's forks to discover what they have as WIP\nis not a good measurement of what work is being done or not. A lot\nof mental energy is being spent in this area.\n\nSaying \"What I see in this person's fork isn't good enough\" is not\na reason to move forward on your own. WIP work is WORK IN PROGRESS\nand is not assumed to be complete or up to expectation for review\non the list.\n\n> FWIW I have this local change queued on top of this v2, it's all\n> cosmetic, but probably a good idea.\n\nPlease stop motivating current patches by work that you have been\nplaying with in your local tree. You do this frequently but it is\nnot helpful.\n \n> The $(SCALAR_SOURCES) bit is something I missed, but which Victoria\n> didn't in her WIP patch (I stole it from there).\n\nYou also mention this in your cover letter (but is notably absent from\nthe patch itself).\n\nI can only attribute your insistence here as a lack of respect for\nyour fellow contributors. There is no rush for this to happen immediately,\nso the only reason is that you don't trust that it would be done\ncorrectly without you submitting the work.\n\nThanks,\n-Stolee\n"},{"id":"457841","messageId":"220624.86y1xmi5wo.gmgdl@evledraar.gmail.com","threadId":"56797","inReplyTo":"c93c8e75-6c88-ac99-d8c3-1e2e7dd06ea3@github.com","subject":"Re: [PATCH v2 0/1] scalar: move to the top-level, test, CI and \"install\" support","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-06-24T11:59:36Z","receivedAt":"2022-06-24T12:48:31Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Jun 23 2022, Derrick Stolee wrote:\n\n> On 6/23/2022 11:30 AM, Ævar Arnfjörð Bjarmason wrote:\n>> \n>> On Thu, Jun 23 2022, Derrick Stolee wrote:\n>> \n>>> On 6/23/2022 6:26 AM, Ævar Arnfjörð Bjarmason wrote:\n>>>> This one-patch series integrates the \"scalar\" command to the\n>>>> top-level, meaning we build the \"scalar\" binary by default, and run\n>>>> its tests on \"make test\" and in CI. We'll also build and test its\n>>>> documentation. We now also have \"install\" support, both for the\n>>>> program and its docs, but you'll need to:\n>>>>\n>>>> make INSTALL_SCALAR=Y\n>>>>\n>>>> I'm sending this out now to avoid needless duplicate work.\n>>>\n>>> As mentioned on the list earlier, Victoria is taking over the\n>>> remaining work to complete the Scalar project. Nothing has been\n>>> sent to the list because we didn't want to cause a distraction\n>>> from the release window.\n>> \n>> I was on the fence about sending this out, but given that the \"CI\"\n>> thread was going on until the start of that window, and wanting to save\n>> her the work of re-discovering the subtle issues with the integration\n>> I'd already fixed I thought it was better ot send it out.\n>\n> The CI thread was halted not just because of the release window,\n> but because of a change in who was doing the work and taking time\n> to revisit the plan of record. This was communicated on-list [1].\n\nPart of this is me reading between the lines, but as mentioned in the CL\nI thought it had more to do with the bug I pointed out on the CI\nseries's 2/2.\n\n> [1] https://lore.kernel.org/git/nycvar.QRO.7.76.6.2206132246010.353@tvgsbejvaqbjf.bet/\n>\n> The point is that the end goal is still \"Get Scalar out of contrib/\"\n> but the questions that are still being worked out are things like:\n>\n> 1. _When_ do we integrate Scalar into CI?\n> 2. _When_ do we move Scalar out of contrib/?\n> 3. _When_ do we implement the remaining Scalar features?\n>\n> All three of these questions depend on what the final result will\n> look like, and Victoria is still working on developing her own\n> opinion on that before presenting it to the list in a complete\n> form.\n>\n> Your patch here is interrupting that process.\n\nCoordination & collaboration can be difficult, but generally speaking we\ntry to use the ML as a means to decide how topics move forward, and for\ncoordination.\n\nIn this case I understand that there was off-list discussion about how a\npart of the codebase should be moving forward among a group of\nco-workers who contribute to git.\n\nI don't think there's anything wrong with that, but this really seems\nlike an overly hostile response to someone who wasn't part of that\nprocess.\n\nEspecially as I'm not insisting that I be the one to drive anything\nforward on the scalar topic, I think it makes much more sense that it's\nVictoria.\n\nThis patch is just offered as a way to help that effort along. Perhaps\nshe'd ack it as-is, find it useful as it reveals some edge cases she\ndidn't know about, or drop it and go for some other approach. Whatever\ngets us to the end-goal sooner than later is fine by me.\n\nOnce you \"git rm\" the scalar Makefile there's not really a lot of ways\nyou can go other than something approximating the upthread patch. Given\nthat some of the edge cases are tricky I deemed it worthwhile to share\nit.\n\n>>> Victoria is taking time to incorporate your previous thoughts on\n>>> how Scalar is built and its location in the codebase and create\n>>> a complete narrative of how to get from our current state to that\n>>> point.\n>> \n>> I wasn't sure she was even aware of it, and given that the WIP patch I\n>> saw in my \"git fetch\" was pretty much a subset of the upthread v1 it\n>> seemed that there was needless duplicate work going on.\n>> \n>> It seemed clear that that WIP patch was attempting to head in the same\n>> direction, but hadn't yet discovered some of the hurdles with\n>> e.g. documentation building & installation that I'd fixed\n>> already. There's also the CMake integration, which I finished up for\n>> this v2.\n>\n> Poking around in people's forks to discover what they have as WIP\n> is not a good measurement of what work is being done or not. A lot\n> of mental energy is being spent in this area.\n>\n> Saying \"What I see in this person's fork isn't good enough\" is not\n> a reason to move forward on your own. WIP work is WORK IN PROGRESS\n> and is not assumed to be complete or up to expectation for review\n> on the list.\n\nI'm not saying that, but \"you seem to be trying to do X, and may or may\nnot be aware of a past patch that does X, here is is in case it help!.\n\n>> FWIW I have this local change queued on top of this v2, it's all\n>> cosmetic, but probably a good idea.\n>\n> Please stop motivating current patches by work that you have been\n> playing with in your local tree. You do this frequently but it is\n> not helpful.\n\nAfter I submit patches to the ML I tend to \"bump\" the branch I sent it\nfrom, that range-diff was against the submitted v2 to local changes I\napplied afterwards, having missed some spots in the initial submission.\n\nI'm not clear on what part of this you're taking issue with...\n\n>> The $(SCALAR_SOURCES) bit is something I missed, but which Victoria\n>> didn't in her WIP patch (I stole it from there).\n>\n> You also mention this in your cover letter (but is notably absent from\n> the patch itself).\n\n...yes, hence the \"local change queued on top\" above, it's a minor\ndetail I noticed after submitting the patch.\n\n> I can only attribute your insistence here as a lack of respect for\n> your fellow contributors. [...]\n\nI really think that's unhelpful. Let's try to assume some mutual good\nintent. I think I've explained my motivations above.\n\n> [...]There is no rush for this to happen immediately, so the only\n> reason is that you don't trust that it would be done correctly without\n> you submitting the work.\n\nSome of the edge cases in the Makefile integration are subtle. I trust\nthat someone looking at it would probably discover those eventually. I\nthink it took me about a day of poking around, much of which was a slow\nping-pong with the CI. So someone with a local Windows box could do it\nfaster.\n\nBut if I'd have gotten a patch from my future self with all the learned\nedge cases beforehand I'd have appreciated, so I figured I'd send it in.\n"},{"id":"457861","messageId":"xmqqzgi1u3rt.fsf@gitster.g","threadId":"56797","inReplyTo":"220624.86y1xmi5wo.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH v2 0/1] scalar: move to the top-level, test, CI and \"install\" support","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-06-24T21:53:42Z","receivedAt":"2022-06-24T21:53:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> Especially as I'm not insisting that I be the one to drive anything\n> forward on the scalar topic, I think it makes much more sense that it's\n> Victoria.\n>\n> This patch is just offered as a way to help that effort along. Perhaps\n> she'd ack it as-is, find it useful as it reveals some edge cases she\n> didn't know about, or drop it and go for some other approach. Whatever\n> gets us to the end-goal sooner than later is fine by me.\n\nCareful ...\n\n> Once you \"git rm\" the scalar Makefile there's not really a lot of ways\n> you can go other than something approximating the upthread patch. Given\n> that some of the edge cases are tricky I deemed it worthwhile to share\n> it.\n> ...\n> I'm not saying that, but \"you seem to be trying to do X, and may or may\n> not be aware of a past patch that does X, here is is in case it help!.\n> ...\n> Some of the edge cases in the Makefile integration are subtle. I trust\n> that someone looking at it would probably discover those eventually.\n\n... Even though you ask to assume good intent, an attitude shown by\nrepeated \"I've done that, I already know *the* solution to the\nproblem you are trying to solve, and you can learn from what I did\"\ngives recipients quite an impression different from what you intend\nto give.\n\n> But if I'd have gotten a patch from my future self with all the learned\n> edge cases beforehand I'd have appreciated, so I figured I'd send it in.\n\nIn any case, if you stop sending *replacement* patches in response\nto others' work, that alone will reduce the friction people around\nyou are feeling quite a lot. A replacement patch is a horrible and\ninefficient way to comment on others' work, if your goal is to be\nunderstood. The recipient would be forced to read and think like\nyou did, making comparison between the two approaches themselves if\nthey want to see if it is worth to use the replacement.\n\nMaking comparison between approaches, arguing for the alternative\napproach to be better, and proposing the alternative to be taken, is\nthe responsibility of the party who is suggesting alternative\napproaches, not that of the recipient.\n\nFor that reason, if you want to say something to other people's\nwork, you are better off doing that either by commenting in-line\n(i.e. annotating their code you want to comment on), or if you\nabsolutely need to talk in code, or by giving an incremental update\nonce their work solidifies (i.e. pointing out a specific issue in\nthe existing code, explaining why it is a problem worth addressing,\nand offering an update---just like when you are fixing a bug).\n\nThanks.\n"}]}