Define lockable files, make read-only in working copy#1870
Merged
Conversation
Also add functions to fix writeable flag based on these patterns
Also requires dropping recursive option which is fine since always recursive
…laces `locking` still depends on `config` but will be refactored out later
Still using strings.Replace, this doesn’t justify using text/template
Define lockable files, make read-only in working copy when not locked
# Conflicts: # locking/locks.go
ttaylorr
reviewed
Jan 18, 2017
Contributor
ttaylorr
left a comment
There was a problem hiding this comment.
Thanks for tackling those merge conflicts 👍 . I'm on board with this, but I'll defer to @technoweenie's judgement before approving this into master.
technoweenie
approved these changes
Jan 18, 2017
chrisd8088
added a commit
to chrisd8088/git-lfs
that referenced
this pull request
Feb 28, 2025
As of Go 1.24, if the Go version number in the "go.mod" file is set to 1.24 or higher, the "go vet" command now reports misuses of non-constant strings as format strings. In previous commits in this PR we have now resolved all but one of the instances where we provided a non-constant string as a format string. The remaining instance is our use of the Errorf() function of the "fmt" package from the Go standard library at the end of the Combine() function in our "errors" package. The Combine() function was added to our custom "errors" package in commit 08e3e5b of PR git-lfs#1870, originally for use in the "locking" package. This function merges multiple errors into a single error by concatenating their error messages with a newline separator character between each original message. In Go 1.20 the Join() function was added to the "errors" package of the Go standard library, and it performs the same concatenation of error messages as our Combine() function, including the use of a newline character as a separator between the original messages, except that it delays the concatenation until the Error() method is called. To resolve the remaining case where we pass a non-constant string as a format string, we remove the Combine() function from our "errors" package and replace it with a Join() function that simply invokes the Join() function of the standard library's "errors" package, which we can expect to be defined as we currently require the use of at least Go 1.21 (per the Go version specified in our "go.mod" file). To make this change, we alias the standard library's "errors" package as "goerrors", following the pattern established in our "lfshttp" package where we use that alias as well. We then revise the two callers of our Combine() function to make use of the new Join() function instead. One of these callers is the startConnection() function in our "ssh" package, which simply joins two errors, and the other caller is the FixLockableFileWriteFlags() method of the Client structure in our "locking" package, which collects zero or more errors while iterating over a list of files and repeatedly calling another method. Because the Join() functions accept variadic arguments rather than an array of errors, we revise these callers to pass the errors they intend to concatenate as direct arguments rather than in an array. For the FixLockableFileWriteFlags() method this means that as it iterates over its list of files, if any errors occur we immediately call the Join() function to add them to the error value we return at the end of the function, whereas previously we appended the errors to an array, and then called the Combine() function after the loop exited to concatenate any errors in the array into the function's final error return value.
chrisd8088
added a commit
to chrisd8088/git-lfs
that referenced
this pull request
Mar 5, 2025
As of Go 1.24, if the Go version number in the "go.mod" file is set to 1.24 or higher, the "go vet" command now reports misuses of non-constant strings as format strings. In previous commits in this PR we have now resolved all but one of the instances where we provided a non-constant string as a format string. The remaining instance is our use of the Errorf() function of the "fmt" package from the Go standard library at the end of the Combine() function in our "errors" package. The Combine() function was added to our custom "errors" package in commit 08e3e5b of PR git-lfs#1870, originally for use in the "locking" package. This function merges multiple errors into a single error by concatenating their error messages with a newline separator character between each original message. In Go 1.20 the Join() function was added to the "errors" package of the Go standard library, and it performs the same concatenation of error messages as our Combine() function, including the use of a newline character as a separator between the original messages, except that it delays the concatenation until the Error() method is called. To resolve the remaining case where we pass a non-constant string as a format string, we remove the Combine() function from our "errors" package and replace it with a Join() function that simply invokes the Join() function of the standard library's "errors" package, which we can expect to be defined as we currently require the use of at least Go 1.21 (per the Go version specified in our "go.mod" file). To make this change, we alias the standard library's "errors" package as "goerrors", following the pattern established in our "lfshttp" package where we use that alias as well. We then revise the two callers of our Combine() function to make use of the new Join() function instead. One of these callers is the startConnection() function in our "ssh" package, which simply joins two errors, and the other caller is the FixLockableFileWriteFlags() method of the Client structure in our "locking" package, which collects zero or more errors while iterating over a list of files and repeatedly calling another method. Because the Join() functions accept variadic arguments rather than an array of errors, we revise these callers to pass the errors they intend to concatenate as direct arguments rather than in an array. For the FixLockableFileWriteFlags() method this means that as it iterates over its list of files, if any errors occur we immediately call the Join() function to add them to the error value we return at the end of the function, whereas previously we appended the errors to an array, and then called the Combine() function after the loop exited to concatenate any errors in the array into the function's final error return value.
chrisd8088
added a commit
to chrisd8088/git-lfs
that referenced
this pull request
May 15, 2025
In PR git-lfs#1870, which was based on the work in PR git-lfs#1822, we introduced support for "lockable" files whose permissions would be managed by several new Git LFS commands such as "git lfs post-checkout" and "git lfs post-merge", both of which we register as Git hooks. In commit 585befa of PRs git-lfs#1822 and git-lfs#1870 we added manual pages for these two commands. (Although these PRs also introduced the "git lfs post-commit" command, they did not include a manual page for it, so we added one belatedly in commit ae5d5b0 of PR git-lfs#4052.) From the time it was written, our git-lfs-post-checkout(1) manual page has indicated that the command accepts three arguments. This accurately reflects the fact that Git will invoke our hook command with parameters specifying the SHAs from before and after the checkout operation, plus a status flag indicating whether a branch checkout was performed or not. However, our manual page does not explain what values these arguments are expected to contain, other than by dint of the names given to the arguments in the page's synopsis section. We therefore expand our description of the "git lfs post-checkout" command to detail the values Git will provide in each of the three arguments. We also take the opportunity to adjust the name of the second argument to clarify that we expect this parameter to contain a Git revision SHA and not the name of a Git reference such as a branch or tag. As well, we replace the underscores in the argument names with hyphens to match the style we use in all our other manual pages for the names of command parameters.
chrisd8088
added a commit
to chrisd8088/git-lfs
that referenced
this pull request
May 15, 2025
In PR git-lfs#1870, which was based on the work in PR git-lfs#1822, we introduced support for "lockable" files whose permissions would be managed by several new Git LFS commands such as "git lfs post-checkout" and "git lfs post-merge", both of which we register as Git hooks. In commit 585befa of PRs git-lfs#1822 and git-lfs#1870 we added manual pages for these two commands. (Although these PRs also introduced the "git lfs post-commit" command, they did not include a manual page for it, so we added one belatedly in commit ae5d5b0 of PR git-lfs#4052.) From the time it was written, our git-lfs-post-merge(1) manual page has indicated that the command accepts a single "<is_squash>" argument. This accurately reflects the fact that Git will invoke our hook command with a single flag parameter that indicates whether or not a squash merge was performed. However, our "git lfs post-merge" command disregards this parameter entirely, so we adjust our manual page now to clarify that this command requires no arguments and ignores the one provided by Git.
chrisd8088
added a commit
to chrisd8088/git-lfs
that referenced
this pull request
Nov 26, 2025
Since commit 3652681 of PR git-lfs#1870 the assert_attributes_count() assertion function in our shell test library has used the grep(1) and wc(1) commands to count the number of lines in the ".gitattributes" file which match a given pattern. We now simplify this operation by using the -c option of the "grep" command to count the number of matching lines instead of a separate "wc" command. We also replace the "!=" comparison operator in the subsequent shell test with the -eq operator, since we expect both of its operands to be integers. These changes slightly simplify the steps in the assert_attributes_count() function, and also bring them into alignment with the many similar commands throughout our test suite that we have revised or corrected in prior commits in this PR. Note that unlike those other instances where we make use of the "grep" command's -c option, in this case the command runs in a subshell within a simple assignment rather than within a "[" shell builtin, so we need to append a "true" command following an "||" OR list operator. This is required to handle the case where the "grep" command finds no lines in its input which match the given pattern. The command will output a line count of "0" to its standard error file descriptor, but will also return an exit code of 1, indicating that the command did not find any matches. Our tests always enable the "errexit" shell attribute using the "set -e" command, and so this would cause the test which called the assert_attributes_count() to fail, because the "grep" command's exit status would be treated as the status of the simple variable assignment. We therefore need to append an "||" operator and a "true" command so that the subshell will always return a zero exit status. By contrast, all our other uses of the "grep" command with the -c option occur in subshells within "[" shell builtins, and so the commands' exit statuses do not supplant the statuses of the "[" builtins themselves.
chrisd8088
added a commit
to chrisd8088/git-lfs
that referenced
this pull request
Nov 26, 2025
Since commit 3652681 of PR git-lfs#1870 the assert_attributes_count() assertion function in our shell test library has used the grep(1) and wc(1) commands to count the number of lines in the ".gitattributes" file which match a given pattern. We now simplify this operation by using the -c option of the "grep" command to count the number of matching lines instead of a separate "wc" command. We also replace the "!=" comparison operator in the subsequent shell test with the -eq operator, since we expect both of its operands to be integers. These changes slightly simplify the steps in the assert_attributes_count() function, and also bring them into alignment with the many similar commands throughout our test suite that we have revised or corrected in prior commits in this PR. Note that unlike those other instances where we make use of the "grep" command's -c option, in this case the command runs in a subshell within a simple assignment rather than within a "[" shell builtin, so we need to append a "true" command following an "||" OR list operator. This is required to handle the case where the "grep" command finds no lines in its input which match the given pattern. The command will output a line count of "0" to its standard error file descriptor, but will also return an exit code of 1, indicating that the command did not find any matches. Our tests always enable the "errexit" shell attribute using the "set -e" command, and so this would cause the test which called the assert_attributes_count() to fail, because the "grep" command's exit status would be treated as the status of the simple variable assignment. We therefore need to append an "||" operator and a "true" command so that the subshell will always return a zero exit status. By contrast, all our other uses of the "grep" command with the -c option occur in subshells within "[" shell builtins, and so the commands' exit statuses do not supplant the statuses of the "[" builtins themselves.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is really just #1822 brought up to date with master. I had quite a hassle resolving all the conflicts since quite a lot of structural changes are being made in master that overlapped with this and I'd rather not have to keep doing that, so would like to merge it.