Skip to content

Define lockable files, make read-only in working copy#1870

Merged
sinbad merged 36 commits intomasterfrom
locking-master-2
Jan 19, 2017
Merged

Define lockable files, make read-only in working copy#1870
sinbad merged 36 commits intomasterfrom
locking-master-2

Conversation

@sinbad
Copy link
Contributor

@sinbad sinbad commented Jan 16, 2017

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.

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
Copy link
Contributor

@ttaylorr ttaylorr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@sinbad sinbad merged commit 200b8f0 into master Jan 19, 2017
@sinbad sinbad deleted the locking-master-2 branch January 19, 2017 12:08
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants