Re: [PATCH v9 03/11] ref-filter: implement an `align` atom
On Sun, Aug 9, 2015 at 9:12 AM, Eric Sunshine wrote:
> On Sat, Aug 8, 2015 at 2:35 AM, Karthik Nayak wrote:
>> On Fri, Aug 7, 2015 at 8:57 AM, Eric Sunshine
>> wrote:
>>> On Wed, Aug 5, 2015 at 2:54 PM, Karthik Nayak wrote:
of the padding to be performed. If the atom length is more than the
padding length then no padding is performed. e.g. to pad a succeeding
atom to the middle with a total padding size of 40 we can do a
>>>
>>> It's odd to have alignment described in terms of "padding" and
>>> "padding length", especially in the case of "center" alignment. It
>>> might be better to rephrase the discussion in terms of field width or
>>> such.
>>>
--format="%(align:middle,40).."
>>
>> Ok this makes sense,
>> I'll rephrase as :
>>
>> `` is the total length of the content with alignment.
>
> This doesn't really make sense. isn't the content length; it's
> the size of the area into which the content will be placed.
>
Will change this.
>> If the atom length is more than the width then no alignment is performed.
>
> What "atom"? I think you mean the content between %(align:) and %(end)
> rather than "atom". The description might be clearer if you actually
> say "content between %(align:) and %(end)" to indicate specifically
> what is being aligned.
Yes, that's what I meant.
>
>> e.g. to align a succeeding atom to the middle with a total width of 40
>> we can do:
>> --format="%(align:middle,40).."
@@ -687,6 +690,29 @@ static void populate_value(struct ref_array_item *ref)
else
v->s = " ";
continue;
+ } else if (starts_with(name, "align:")) {
+ const char *valp = NULL;
>>>
>>> Unnecessary NULL assignment.
>>
>> Thats required for the second skip_prefix and so on.
>> Else we get: "warning: ‘valp’ may be used uninitialized in this
>> function [-Wmaybe-uninitialized]"
>
> Okay, so that's because skip_prefix() is inline, and it doesn't touch
> its "out" argument unless it actually skips the prefix. Ugly, but
> makes sense, although I think this issue would go away if you combined
> the starts_with() and skips_prefix() as suggested earlier.
>
Okay then I'll declare valp prehand to get rid of this and also to
remove redundant, starts_with() and skip_prefix().
+ struct align *align = xmalloc(sizeof(struct
align));
+
+ skip_prefix(name, "align:", &valp);
>>>
>>> You could simplify the code by combining this skip_prefix() with the
>>> earlier starts_with() in the conditional:
>>>
>>> } else if (skip_prefix(name, "align:", &valp)) {
>>> struct align *align = xmalloc(sizeof(struct align));
>>> ...
>>
>> That would require valp to be previously defined. Hence the split.
>
> Yes, it would require declaring 'valp' earlier, but that seems a
> reasonable tradeoff for cleaner, simpler, less redundant code.
>
Yes. will do this.
static void apply_formatting_state(struct ref_formatting_state *state,
struct strbuf *final)
{
- /* More formatting options to be evetually added */
+ if (state->align && state->end) {
+ struct strbuf *value = state->output;
+ int len = 0, buf_len = value->len;
+ struct align *align = state->align;
+
+ if (!value->buf)
+ return;
+ if (!is_utf8(value->buf)) {
+ len = value->len - utf8_strwidth(value->buf);
>>>
>>> What is this doing, exactly? If the string is *not* utf-8, then you're
>>> asking it for its utf-8 width. Am I reading that correctly?
>>>
>>> Also, what is 'len' supposed to represent? I guess you want it to be
>>> the difference between the byte length and the display length, but the
>>> name 'len' doesn't convey that at all, nor does it help the reader
>>> understand the code below where you do the actual formatting.
>>>
>>> In fact, if I'm reading this correctly, then 'len' is always zero in
>>> your tests (because the tests never trigger this conditional), so this
>>> functionality is never being exercised.
>>
>> There shouldn't be a "!" there, will change.
>> I guess 'utf8_compensation' would be a better name.
>
> Definitely better than 'len'.
>
+ else if (align->align_type == ALIGN_MIDDLE) {
+ int right = (align->align_value - buf_len)/2;
+ strbuf_addf(final, "%*s%-*s", align->align_value -
right + len,
+ value->buf, right, "");
>>>
>>> An aesthetic aside: When (align_value - buf_len) is an odd number,
>>> this implementation favors placing more whitespace to the left of the
>>> string, and less to the right. In practice, this often tends to look a
>>> bit more awkward than the inverse of placing more whitespace to the
>>> rig
Re: [PATCH v2] worktree: list operation
Michael Rappazzo writes:
> @@ -36,7 +37,7 @@ static int prune_worktree(const char *id, struct strbuf
> *reason)
> fd = open(git_path("worktrees/%s/gitdir", id), O_RDONLY);
> if (fd < 0) {
> strbuf_addf(reason, _("Removing worktrees/%s: unable to read
> gitdir file (%s)"),
> - id, strerror(errno));
> + id, strerror(errno));
There are still a lot of spurious whitespace changes.
Andreas.
--
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 03/11] ref-filter: implement an `align` atom
On Sat, Aug 8, 2015 at 2:35 AM, Karthik Nayak wrote:
> On Fri, Aug 7, 2015 at 8:57 AM, Eric Sunshine wrote:
>> On Wed, Aug 5, 2015 at 2:54 PM, Karthik Nayak wrote:
>>> of the padding to be performed. If the atom length is more than the
>>> padding length then no padding is performed. e.g. to pad a succeeding
>>> atom to the middle with a total padding size of 40 we can do a
>>
>> It's odd to have alignment described in terms of "padding" and
>> "padding length", especially in the case of "center" alignment. It
>> might be better to rephrase the discussion in terms of field width or
>> such.
>>
>>> --format="%(align:middle,40).."
>
> Ok this makes sense,
> I'll rephrase as :
>
> `` is the total length of the content with alignment.
This doesn't really make sense. isn't the content length; it's
the size of the area into which the content will be placed.
> If the atom length is more than the width then no alignment is performed.
What "atom"? I think you mean the content between %(align:) and %(end)
rather than "atom". The description might be clearer if you actually
say "content between %(align:) and %(end)" to indicate specifically
what is being aligned.
> e.g. to align a succeeding atom to the middle with a total width of 40
> we can do:
> --format="%(align:middle,40).."
>>> @@ -687,6 +690,29 @@ static void populate_value(struct ref_array_item *ref)
>>> else
>>> v->s = " ";
>>> continue;
>>> + } else if (starts_with(name, "align:")) {
>>> + const char *valp = NULL;
>>
>> Unnecessary NULL assignment.
>
> Thats required for the second skip_prefix and so on.
> Else we get: "warning: ‘valp’ may be used uninitialized in this
> function [-Wmaybe-uninitialized]"
Okay, so that's because skip_prefix() is inline, and it doesn't touch
its "out" argument unless it actually skips the prefix. Ugly, but
makes sense, although I think this issue would go away if you combined
the starts_with() and skips_prefix() as suggested earlier.
>>> + struct align *align = xmalloc(sizeof(struct align));
>>> +
>>> + skip_prefix(name, "align:", &valp);
>>
>> You could simplify the code by combining this skip_prefix() with the
>> earlier starts_with() in the conditional:
>>
>> } else if (skip_prefix(name, "align:", &valp)) {
>> struct align *align = xmalloc(sizeof(struct align));
>> ...
>
> That would require valp to be previously defined. Hence the split.
Yes, it would require declaring 'valp' earlier, but that seems a
reasonable tradeoff for cleaner, simpler, less redundant code.
>>> static void apply_formatting_state(struct ref_formatting_state *state,
>>> struct strbuf *final)
>>> {
>>> - /* More formatting options to be evetually added */
>>> + if (state->align && state->end) {
>>> + struct strbuf *value = state->output;
>>> + int len = 0, buf_len = value->len;
>>> + struct align *align = state->align;
>>> +
>>> + if (!value->buf)
>>> + return;
>>> + if (!is_utf8(value->buf)) {
>>> + len = value->len - utf8_strwidth(value->buf);
>>
>> What is this doing, exactly? If the string is *not* utf-8, then you're
>> asking it for its utf-8 width. Am I reading that correctly?
>>
>> Also, what is 'len' supposed to represent? I guess you want it to be
>> the difference between the byte length and the display length, but the
>> name 'len' doesn't convey that at all, nor does it help the reader
>> understand the code below where you do the actual formatting.
>>
>> In fact, if I'm reading this correctly, then 'len' is always zero in
>> your tests (because the tests never trigger this conditional), so this
>> functionality is never being exercised.
>
> There shouldn't be a "!" there, will change.
> I guess 'utf8_compensation' would be a better name.
Definitely better than 'len'.
>>> + else if (align->align_type == ALIGN_MIDDLE) {
>>> + int right = (align->align_value - buf_len)/2;
>>> + strbuf_addf(final, "%*s%-*s", align->align_value -
>>> right + len,
>>> + value->buf, right, "");
>>
>> An aesthetic aside: When (align_value - buf_len) is an odd number,
>> this implementation favors placing more whitespace to the left of the
>> string, and less to the right. In practice, this often tends to look a
>> bit more awkward than the inverse of placing more whitespace to the
>> right, and less to the left (but that again is subjective).
>
> I know that, maybe we could add an additional padding to even out the value
> given?
I don't understand your question. I was merely suggesting (purely
subjectively), for the "odd length" case, putting the extra space
after the centered text rather than before it. For instance:
int left = (align->align_value
Re: [PATCH v5 1/2] worktrees: add find_shared_symref
On Mon, Aug 3, 2015 at 7:06 PM, Eric Sunshine wrote:
> On Mon, Aug 3, 2015 at 2:48 PM, David Turner wrote:
>> Add a new function, find_shared_symref, which contains the heart of
>> die_if_checked_out, but works for any symref, not just HEAD. Refactor
>> die_if_checked_out to use the same infrastructure as
>> find_shared_symref.
>>
>> Soon, we will use find_shared_symref to protect notes merges in
>> worktrees.
>>
>> Signed-off-by: David Turner
>> ---
>> diff --git a/branch.c b/branch.c
>> index c85be07..d2b3586 100644
>> --- a/branch.c
>> +++ b/branch.c
>> @@ -349,31 +350,53 @@ static void check_linked_checkout(const char *branch,
>> const char *id)
>> strbuf_addstr(&gitdir, get_git_common_dir());
>> skip_prefix(branch, "refs/heads/", &branch);
>> strbuf_strip_suffix(&gitdir, ".git");
>> - die(_("'%s' is already checked out at '%s'"), branch, gitdir.buf);
In addition to the other issues already mentioned, this one is a bit
more serious (though still rather superficial):
With this change, the skip_prefix(branch, "refs/heads/", &branch)
becomes meaningless. It was used by the die() to provide a nicer
looking error message, however, 'branch' now is never used after
skip_prefix(). More below...
>> +
>> + strbuf_release(&path);
>> + strbuf_release(&sb);
>> + return strbuf_detach(&gitdir, NULL);
>> done:
>> strbuf_release(&path);
>> strbuf_release(&sb);
>> strbuf_release(&gitdir);
>> +
>> + return NULL;
>> }
>
> This would be cleaner and less redundant if you assign the existing
> location to a variable and just fall through to the 'done' label:
>
> char *existing = NULL;
> ...
> skip_prefix(branch, "refs/heads/", &branch);
> strbuf_strip_suffix(&gitdir, ".git");
> existing = strbuf_detach(&gitdir, NULL);
> done:
> strbuf_release(&path);
> strbuf_release(&sb);
> strbuf_release(&gitdir);
> return existing;
>
> There's no worry that the "existing" path will be clobbered by
> strbuf_release(&gitdir) since it's been detached already (and it's
> safe to release the strbuf without affecting what has been detached
> from it).
>
>> -void die_if_checked_out(const char *branch)
>> +char *find_shared_symref(const char *symref, const char *target)
>> {
>> struct strbuf path = STRBUF_INIT;
>> DIR *dir;
>> struct dirent *d;
>> + char *existing;
>>
>> - check_linked_checkout(branch, NULL);
>> + if ((existing = find_linked_symref(symref, target, NULL)))
>> + return existing;
>>
>> strbuf_addf(&path, "%s/worktrees", get_git_common_dir());
>> dir = opendir(path.buf);
>> strbuf_release(&path);
>> if (!dir)
>> - return;
>> + return NULL;
>>
>> while ((d = readdir(dir)) != NULL) {
>> if (!strcmp(d->d_name, ".") || !strcmp(d->d_name, ".."))
>> continue;
>> - check_linked_checkout(branch, d->d_name);
>> + existing = find_linked_symref(symref, target, d->d_name);
>> + if (existing) {
>> + closedir(dir);
>> + return existing;
>
> For consistency with code nearby, this could have been handled by
> adding a 'done' label above the closedir() below and jumping to it
> from here, and then 'return existing'.
>
>> + }
>> }
>> closedir(dir);
>> +
>> + return NULL;
>> +}
>> +
>> +void die_if_checked_out(const char *branch)
>> +{
>> + char *existing;
>> +
>> + existing = find_shared_symref("HEAD", branch);
>> + if (existing)
>> + die(_("'%s' is already checked out at '%s'"), branch,
>> existing);
Unlike the original die() in check_linked_checkout() which printed the
branch named shortened by skip_prefix(), this one still prints the
long-form branch name. An easy fix would be to move the skip_prefix()
invocation down to here to skip the "refs/heads/" prefix just before
die().
>> }
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] config.mak.uname: Cygwin: Use renames for creation
On Sat, Aug 08, 2015 at 09:06:28PM +, brian m. carlson wrote: > On Sat, Aug 08, 2015 at 04:47:46PM -0400, Mark Levedahl wrote: > > On 08/07/2015 04:30 PM, Adam Dinwoodie wrote: > > >When generating build options for Cygwin, enable > > >OBJECT_CREATION_USES_RENAMES. This is necessary to use Git on Windows > > >shared directories, and is already enabled for the MinGW and plain > > >Windows builds. > > > > I've been supporting use of git on cygwin since about 2008, this issue has > > never risen that I know. Whatever issue is being patched around here, if > > truly repeatable, should be handled by the cygwin dll as that code is > > focused on providing full linux compatibility. If git on linux does need > > this patch, git on cygwin should not, either. So, I vote against this. There has been recent and historical discussion on the Cygwin mailing list about this problem, as well as in other places like Stack Overflow. I've put a link to one of those discussions on the Cygwin mailing list in the original patch email. I can also see some discussiions on this list that seem related (search archives for "failed to read delta-pack base object" and "Cygwin"). It may be the technically correct approach that the Cygwin library ought to fix this, and indeed some improvements have been made in this area. However given the limited interfaces that Windows offers here, a final fix is very unlikely to come any time soon, so this patch is the pragmatic solution. I do not see any difference between the situation here and the situation for MinGW, which is fundamentally a Cygwin fork, but which already has this build option set for it in config.mak.uname. > We've gotten a lot of users on the list who ask why their Git > directories on shared drives aren't working (or are broken in some way). > Since I don't use Windows, let me ask: does the Cygwin DLL handle > link(2) properly on shared drives, and if not, would this patch help it > do so? I can imagine that perhaps SMB doesn't support the necessary > operations to make a POSIX link(2) work properly. I'd need to go back to the Cygwin list to get a definite answer, but as I understand it, yes, this is is exactly the problem -- quoting Corinna, one of the Cygwin project leads, "The MS NFS is not very reliable in keeping up with changes to metadata." We have verified that setting `core.createobject rename` resolves the problem for people who are seeing it, which very strongly implies that this build option would solve the problem similarly, but would fix it for all users, not just those who spend enough time investigating the problem to find that setting. Adam -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] worktree: list operation
I am attempting to add the 'git worktree list' command. I don't have a lot of c experience, so please double check that I am not missing something important. Sorry about publishing the first version too soon. Michael Rappazzo (1): worktree: list operation Documentation/git-worktree.txt | 9 - builtin/worktree.c | 80 ++ t/t2027-worktree-list.sh | 68 +++ 3 files changed, 150 insertions(+), 7 deletions(-) create mode 100755 t/t2027-worktree-list.sh -- 2.5.0 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] worktree: list operation
'git worktree list' will list the main worktree followed by any linked
worktrees which were created using 'git worktree add'. The option
'--main-only' will restrict the list to only the main worktree.
---
Documentation/git-worktree.txt | 9 -
builtin/worktree.c | 84 ++
t/t2027-worktree-list.sh | 68 ++
3 files changed, 152 insertions(+), 9 deletions(-)
create mode 100755 t/t2027-worktree-list.sh
diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index 3387e2f..2b6b543 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -11,6 +11,7 @@ SYNOPSIS
[verse]
'git worktree add' [-f] [--detach] [-b ] []
'git worktree prune' [-n] [-v] [--expire ]
+'git worktree list' [--main-only]
DESCRIPTION
---
@@ -59,6 +60,10 @@ prune::
Prune working tree information in $GIT_DIR/worktrees.
+list::
+
+List the main worktree followed by all of the linked worktrees.
+
OPTIONS
---
@@ -86,6 +91,9 @@ OPTIONS
With `prune`, do not remove anything; just report what it would
remove.
+--main-only::
+ With `list`, only list the main worktree.
+
-v::
--verbose::
With `prune`, report all removals.
@@ -167,7 +175,6 @@ performed manually, such as:
- `remove` to remove a linked worktree and its administrative files (and
warn if the worktree is dirty)
- `mv` to move or rename a worktree and update its administrative files
-- `list` to list linked worktrees
- `lock` to prevent automatic pruning of administrative files (for instance,
for a worktree on a portable device)
diff --git a/builtin/worktree.c b/builtin/worktree.c
index 6a264ee..8c4a82a 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -10,6 +10,7 @@
static const char * const worktree_usage[] = {
N_("git worktree add [] "),
N_("git worktree prune []"),
+ N_("git worktree list []"),
NULL
};
@@ -36,7 +37,7 @@ static int prune_worktree(const char *id, struct strbuf
*reason)
fd = open(git_path("worktrees/%s/gitdir", id), O_RDONLY);
if (fd < 0) {
strbuf_addf(reason, _("Removing worktrees/%s: unable to read
gitdir file (%s)"),
- id, strerror(errno));
+ id, strerror(errno));
return 1;
}
len = st.st_size;
@@ -59,7 +60,7 @@ static int prune_worktree(const char *id, struct strbuf
*reason)
* accessed since?
*/
if (!stat(git_path("worktrees/%s/link", id), &st_link) &&
- st_link.st_nlink > 1)
+ st_link.st_nlink > 1)
return 0;
if (st.st_mtime <= expire) {
strbuf_addf(reason, _("Removing worktrees/%s: gitdir
file points to non-existent location"), id);
@@ -187,11 +188,11 @@ static int add_worktree(const char *path, const char
**child_argv)
name = worktree_basename(path, &len);
strbuf_addstr(&sb_repo,
- git_path("worktrees/%.*s", (int)(path + len - name),
name));
+ git_path("worktrees/%.*s", (int)(path + len -
name), name));
len = sb_repo.len;
if (safe_create_leading_directories_const(sb_repo.buf))
die_errno(_("could not create leading directories of '%s'"),
- sb_repo.buf);
+ sb_repo.buf);
while (!stat(sb_repo.buf, &st)) {
counter++;
strbuf_setlen(&sb_repo, len);
@@ -218,14 +219,14 @@ static int add_worktree(const char *path, const char
**child_argv)
strbuf_addf(&sb_git, "%s/.git", path);
if (safe_create_leading_directories_const(sb_git.buf))
die_errno(_("could not create leading directories of '%s'"),
- sb_git.buf);
+ sb_git.buf);
junk_work_tree = xstrdup(path);
strbuf_reset(&sb);
strbuf_addf(&sb, "%s/gitdir", sb_repo.buf);
write_file(sb.buf, 1, "%s\n", real_path(sb_git.buf));
write_file(sb_git.buf, 1, "gitdir: %s/worktrees/%s\n",
- real_path(get_git_common_dir()), name);
+ real_path(get_git_common_dir()), name);
/*
* This is to keep resolve_ref() happy. We need a valid HEAD
* or is_git_directory() will reject the directory. Moreover, HEAD
@@ -280,9 +281,9 @@ static int add(int ac, const char **av, const char *prefix)
struct option options[] = {
OPT__FORCE(&force, N_("checkout even if already
checked out in other worktree")),
OPT_STRING('b', NULL, &new_branch, N_("branch"),
- N_("create a new branch")),
+ N_("create a new branch")),
OPT_STRING('B', NULL, &new_branch_force, N_("branch"),
-
Re: [PATCH] worktree: list operation
Withdrawn -- I staged but did not amend the final commit. I will
adjust and resend.
On Sat, Aug 8, 2015 at 4:34 PM, Michael Rappazzo wrote:
> 'git worktree list' will list the main worktree followed by any linked
> worktrees which were created using 'git worktree add'. The option
> '--main-only' will restrict the list to only the main worktree.
> ---
> Documentation/git-worktree.txt | 9 -
> builtin/worktree.c | 80
> ++
> t/t2027-worktree-list.sh | 68 +++
> 3 files changed, 150 insertions(+), 7 deletions(-)
> create mode 100755 t/t2027-worktree-list.sh
>
> diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
> index 3387e2f..39b1330 100644
> --- a/Documentation/git-worktree.txt
> +++ b/Documentation/git-worktree.txt
> @@ -11,6 +11,7 @@ SYNOPSIS
> [verse]
> 'git worktree add' [-f] [--detach] [-b ] []
> 'git worktree prune' [-n] [-v] [--expire ]
> +'git worktree list' [--primary]
>
> DESCRIPTION
> ---
> @@ -59,6 +60,10 @@ prune::
>
> Prune working tree information in $GIT_DIR/worktrees.
>
> +list::
> +
> +List the primary worktree followed by all of the linked worktrees.
> +
> OPTIONS
> ---
>
> @@ -86,6 +91,9 @@ OPTIONS
> With `prune`, do not remove anything; just report what it would
> remove.
>
> +--primary::
> + With `list`, only list the primary worktree.
> +
> -v::
> --verbose::
> With `prune`, report all removals.
> @@ -167,7 +175,6 @@ performed manually, such as:
> - `remove` to remove a linked worktree and its administrative files (and
>warn if the worktree is dirty)
> - `mv` to move or rename a worktree and update its administrative files
> -- `list` to list linked worktrees
> - `lock` to prevent automatic pruning of administrative files (for instance,
>for a worktree on a portable device)
>
> diff --git a/builtin/worktree.c b/builtin/worktree.c
> index 6a264ee..1ac1776 100644
> --- a/builtin/worktree.c
> +++ b/builtin/worktree.c
> @@ -10,6 +10,7 @@
> static const char * const worktree_usage[] = {
> N_("git worktree add [] "),
> N_("git worktree prune []"),
> + N_("git worktree list []"),
> NULL
> };
>
> @@ -36,7 +37,7 @@ static int prune_worktree(const char *id, struct strbuf
> *reason)
> fd = open(git_path("worktrees/%s/gitdir", id), O_RDONLY);
> if (fd < 0) {
> strbuf_addf(reason, _("Removing worktrees/%s: unable to read
> gitdir file (%s)"),
> - id, strerror(errno));
> +id, strerror(errno));
> return 1;
> }
> len = st.st_size;
> @@ -59,7 +60,7 @@ static int prune_worktree(const char *id, struct strbuf
> *reason)
> * accessed since?
> */
> if (!stat(git_path("worktrees/%s/link", id), &st_link) &&
> - st_link.st_nlink > 1)
> +st_link.st_nlink > 1)
> return 0;
> if (st.st_mtime <= expire) {
> strbuf_addf(reason, _("Removing worktrees/%s: gitdir
> file points to non-existent location"), id);
> @@ -187,7 +188,7 @@ static int add_worktree(const char *path, const char
> **child_argv)
>
> name = worktree_basename(path, &len);
> strbuf_addstr(&sb_repo,
> - git_path("worktrees/%.*s", (int)(path + len - name),
> name));
> + git_path("worktrees/%.*s", (int)(path + len -
> name), name));
> len = sb_repo.len;
> if (safe_create_leading_directories_const(sb_repo.buf))
> die_errno(_("could not create leading directories of '%s'"),
> @@ -225,7 +226,7 @@ static int add_worktree(const char *path, const char
> **child_argv)
> strbuf_addf(&sb, "%s/gitdir", sb_repo.buf);
> write_file(sb.buf, 1, "%s\n", real_path(sb_git.buf));
> write_file(sb_git.buf, 1, "gitdir: %s/worktrees/%s\n",
> - real_path(get_git_common_dir()), name);
> + real_path(get_git_common_dir()), name);
> /*
> * This is to keep resolve_ref() happy. We need a valid HEAD
> * or is_git_directory() will reject the directory. Moreover, HEAD
> @@ -280,9 +281,9 @@ static int add(int ac, const char **av, const char
> *prefix)
> struct option options[] = {
> OPT__FORCE(&force, N_("checkout even if already
> checked out in other worktree")),
> OPT_STRING('b', NULL, &new_branch, N_("branch"),
> - N_("create a new branch")),
> + N_("create a new branch")),
> OPT_STRING('B', NULL, &new_branch_force, N_("branch"),
> - N_("create or reset a branch")),
> + N_("create or reset a branch")),
> OPT_BOOL(0,
Re: [PATCH] config.mak.uname: Cygwin: Use renames for creation
On Sat, Aug 08, 2015 at 04:47:46PM -0400, Mark Levedahl wrote: > On 08/07/2015 04:30 PM, Adam Dinwoodie wrote: > >When generating build options for Cygwin, enable > >OBJECT_CREATION_USES_RENAMES. This is necessary to use Git on Windows > >shared directories, and is already enabled for the MinGW and plain > >Windows builds. > I've been supporting use of git on cygwin since about 2008, this issue has > never risen that I know. Whatever issue is being patched around here, if > truly repeatable, should be handled by the cygwin dll as that code is > focused on providing full linux compatibility. If git on linux does need > this patch, git on cygwin should not, either. So, I vote against this. We've gotten a lot of users on the list who ask why their Git directories on shared drives aren't working (or are broken in some way). Since I don't use Windows, let me ask: does the Cygwin DLL handle link(2) properly on shared drives, and if not, would this patch help it do so? I can imagine that perhaps SMB doesn't support the necessary operations to make a POSIX link(2) work properly. -- brian m. carlson / brian with sandals: Houston, Texas, US +1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187 signature.asc Description: Digital signature
Re: [PATCH] config.mak.uname: Cygwin: Use renames for creation
On 08/07/2015 04:30 PM, Adam Dinwoodie wrote: When generating build options for Cygwin, enable OBJECT_CREATION_USES_RENAMES. This is necessary to use Git on Windows shared directories, and is already enabled for the MinGW and plain Windows builds. This problem was reported on the Cygwin mailing list at https://cygwin.com/ml/cygwin/2015-08/msg00102.html (amongst others) and is being applied as a manual patch to the Cygwin builds until the patch is taken here. Reported-by: Peter Rosin Signed-off-by: Adam Dinwoodie --- config.mak.uname | 1 + 1 file changed, 1 insertion(+) diff --git a/config.mak.uname b/config.mak.uname index 943c439..be5cbec 100644 --- a/config.mak.uname +++ b/config.mak.uname @@ -187,6 +187,7 @@ ifeq ($(uname_O),Cygwin) X = .exe UNRELIABLE_FSTAT = UnfortunatelyYes SPARSE_FLAGS = -isystem /usr/include/w32api -Wno-one-bit-signed-bitfield + OBJECT_CREATION_USES_RENAMES = UnfortunatelyNeedsTo endif ifeq ($(uname_S),FreeBSD) NEEDS_LIBICONV = YesPlease I've been supporting use of git on cygwin since about 2008, this issue has never risen that I know. Whatever issue is being patched around here, if truly repeatable, should be handled by the cygwin dll as that code is focused on providing full linux compatibility. If git on linux does need this patch, git on cygwin should not, either. So, I vote against this. Mark -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] worktree: list operation
I am attempting to add the 'git worktree list' command. I don't have a lot of c experience, so please double check that I am not missing something important. Michael Rappazzo (1): worktree: list operation Documentation/git-worktree.txt | 9 - builtin/worktree.c | 80 ++ t/t2027-worktree-list.sh | 68 +++ 3 files changed, 150 insertions(+), 7 deletions(-) create mode 100755 t/t2027-worktree-list.sh -- 2.5.0 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] worktree: list operation
'git worktree list' will list the main worktree followed by any linked
worktrees which were created using 'git worktree add'. The option
'--main-only' will restrict the list to only the main worktree.
---
Documentation/git-worktree.txt | 9 -
builtin/worktree.c | 80 ++
t/t2027-worktree-list.sh | 68 +++
3 files changed, 150 insertions(+), 7 deletions(-)
create mode 100755 t/t2027-worktree-list.sh
diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index 3387e2f..39b1330 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -11,6 +11,7 @@ SYNOPSIS
[verse]
'git worktree add' [-f] [--detach] [-b ] []
'git worktree prune' [-n] [-v] [--expire ]
+'git worktree list' [--primary]
DESCRIPTION
---
@@ -59,6 +60,10 @@ prune::
Prune working tree information in $GIT_DIR/worktrees.
+list::
+
+List the primary worktree followed by all of the linked worktrees.
+
OPTIONS
---
@@ -86,6 +91,9 @@ OPTIONS
With `prune`, do not remove anything; just report what it would
remove.
+--primary::
+ With `list`, only list the primary worktree.
+
-v::
--verbose::
With `prune`, report all removals.
@@ -167,7 +175,6 @@ performed manually, such as:
- `remove` to remove a linked worktree and its administrative files (and
warn if the worktree is dirty)
- `mv` to move or rename a worktree and update its administrative files
-- `list` to list linked worktrees
- `lock` to prevent automatic pruning of administrative files (for instance,
for a worktree on a portable device)
diff --git a/builtin/worktree.c b/builtin/worktree.c
index 6a264ee..1ac1776 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -10,6 +10,7 @@
static const char * const worktree_usage[] = {
N_("git worktree add [] "),
N_("git worktree prune []"),
+ N_("git worktree list []"),
NULL
};
@@ -36,7 +37,7 @@ static int prune_worktree(const char *id, struct strbuf
*reason)
fd = open(git_path("worktrees/%s/gitdir", id), O_RDONLY);
if (fd < 0) {
strbuf_addf(reason, _("Removing worktrees/%s: unable to read
gitdir file (%s)"),
- id, strerror(errno));
+id, strerror(errno));
return 1;
}
len = st.st_size;
@@ -59,7 +60,7 @@ static int prune_worktree(const char *id, struct strbuf
*reason)
* accessed since?
*/
if (!stat(git_path("worktrees/%s/link", id), &st_link) &&
- st_link.st_nlink > 1)
+st_link.st_nlink > 1)
return 0;
if (st.st_mtime <= expire) {
strbuf_addf(reason, _("Removing worktrees/%s: gitdir
file points to non-existent location"), id);
@@ -187,7 +188,7 @@ static int add_worktree(const char *path, const char
**child_argv)
name = worktree_basename(path, &len);
strbuf_addstr(&sb_repo,
- git_path("worktrees/%.*s", (int)(path + len - name),
name));
+ git_path("worktrees/%.*s", (int)(path + len -
name), name));
len = sb_repo.len;
if (safe_create_leading_directories_const(sb_repo.buf))
die_errno(_("could not create leading directories of '%s'"),
@@ -225,7 +226,7 @@ static int add_worktree(const char *path, const char
**child_argv)
strbuf_addf(&sb, "%s/gitdir", sb_repo.buf);
write_file(sb.buf, 1, "%s\n", real_path(sb_git.buf));
write_file(sb_git.buf, 1, "gitdir: %s/worktrees/%s\n",
- real_path(get_git_common_dir()), name);
+ real_path(get_git_common_dir()), name);
/*
* This is to keep resolve_ref() happy. We need a valid HEAD
* or is_git_directory() will reject the directory. Moreover, HEAD
@@ -280,9 +281,9 @@ static int add(int ac, const char **av, const char *prefix)
struct option options[] = {
OPT__FORCE(&force, N_("checkout even if already
checked out in other worktree")),
OPT_STRING('b', NULL, &new_branch, N_("branch"),
- N_("create a new branch")),
+ N_("create a new branch")),
OPT_STRING('B', NULL, &new_branch_force, N_("branch"),
- N_("create or reset a branch")),
+ N_("create or reset a branch")),
OPT_BOOL(0, "detach", &detach, N_("detach HEAD at named
commit")),
OPT_END()
};
@@ -316,6 +317,71 @@ static int add(int ac, const char **av, const char *prefix)
return add_worktree(path, cmd.argv);
}
+static int list(int ac, const char **av, const char *prefix)
+{
+ int primary = 0;
+ struct option options[] = {
+ OP
Re: [PATCH v4 1/3] clone: do not include authentication data in guessed dir
On Fri, Aug 07, 2015 at 01:45:54PM -0700, Junio C Hamano wrote:
> Patrick Steinhardt writes:
>
> > On Wed, Aug 05, 2015 at 12:41:27PM -0700, Junio C Hamano wrote:
> >> Junio C Hamano writes:
> >>
> >> > For completeness, here is what I think the end result (together with
> >> > Peff's series) of the test should look like.
> >> > ...
> >> > Note that ssh://user:passw@rd@host:1234/ and user:passw@rd@host:/
> >> > tests fail for the same reason (finding @ should be greedy, I think).
> >>
> >> And I think this should make it pass. Just remember the last
> >> occurrence of '@' by moving the 'start' every time we see an '@'
> >> sign.
> >>
> >> builtin/clone.c | 11 +--
> >> 1 file changed, 5 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/builtin/clone.c b/builtin/clone.c
> >> index cae288f..5d86439 100644
> >> --- a/builtin/clone.c
> >> +++ b/builtin/clone.c
> >> @@ -160,13 +160,12 @@ static char *guess_dir_name(const char *repo, int
> >> is_bundle, int is_bare)
> >>start += 3;
> >>
> >>/*
> >> - * Skip authentication data.
> >> + * Skip authentication data, if exists.
> >> */
> >> - ptr = start;
> >> - while (ptr < end && !is_dir_sep(*ptr) && *ptr != '@')
> >> - ptr++;
> >> - if (*ptr == '@')
> >> - start = ptr + 1;
> >> + for (ptr = start; ptr < end && !is_dir_sep(*ptr); ptr++) {
> >> + if (*ptr == '@')
> >> + start = ptr + 1;
> >> + }
> >>
> >>/*
> >> * Strip trailing spaces, slashes and /.git
> >
> > I guess it makes sense to skip over @-signs greedily. Is there
> > anything I need to do here or will you squash those changes in?
>
> Sorry but I won't have patience to split the "these need squashing
> in" into multiple pieces and add them to appropriate commits in the
> 5 patch series (counting Peff's two fixes on top of which you built
> your 3 patch series) correctly. You as the original author are much
> better equipped to do so than I am, so I'd appreciate if you can
> reroll them as a 5-patch series.
>
> Among the changes, the fix to builtin/clone.c needs to be squashed
> into your "clone: do not include authentication data in guessed
> dir", and the remainder of the patch to t5603 should go to Peff's
> "clone: add tests for output directory", but with most of them
> marked initially as failing. And then as you fix them in your
> patches, some of the "fail" mark should be dropped.
>
> Thanks.
No problem, just wanted to make sure on how to proceed.
signature.asc
Description: Digital signature
Von James Hartop
Von James Hartop Brondesbury, North West London, England Lieber Freund. Ich bin James William Hartop von Brondesbury, North West London, hier in England. Ich arbeite für Niederlassung der UBS Investment Bank in London. Ich schreibe Ihnen aus meinem Büro, das von einem riesigen Vorteil für uns beide sein wird. In meiner Abteilung, wobei der Geschäftsführer / Leiter der Coverage and Advisory bei der UBS AG (Greater London Regionalbüro), entdeckte ich eine verlassene Summe von £ 15 Millionen britische Pfund Sterling (fünfzehn Millionen britische Pfund) in einem Konto, um angehört einem unserer ausländischen Kunden verstorbenen Herrn Steve Allen, der leider bei einem Autounfall, einschließlich seiner Frau und einzige Tochter verlor sein Leben. Die Wahl der Kontaktaufnahme mit Ihnen ist aus der geographischen Natur, wo Sie leben, vor allem aufgrund der Sensibilität der Transaktion geweckt. Die Bank Beamten haben für keine der Verwandten gewartet zu kommen-up für diese Behauptung, aber niemand hat das getan. Ich persönlich habe schon in der Suche die Verwandten für 4 Jahre jetzt erfolglos, ich suche Ihre Zustimmung an Sie als nächsten Angehörigen / Will Begünstigte an den Verstorbenen dieses Fonds zu präsentieren, so dass die Erlöse aus diesem Konto im Wert von £ 15 Millionen Pfund werden Ihr Bankkonto, wie die nächsten Angehörigen zu spät Mr. Steve Allen übergeben. Dies wird ausgezahlt oder geteilt in diese Prozentsätze, 60% für mich und 40% zu Ihnen. Ich habe alle notwendigen rechtlichen Dokumente, die verwendet werden, um diese Behauptung wir machen werden gesichert. Alles, was ich tun müssen, ist in Ihrem Namen zu den Dokumenten zu füllen und legalisieren sie vor dem Gericht hier, um Sie als berechtigten Empfänger des Fonds zu beweisen. Alles, was ich jetzt verlangen, ist Ihre ehrliche Zusammenarbeit, Verschwiegenheit und Vertrauen, damit wir sieht diese Transaktion durch. Ich garantiere Ihnen, dass dies unter einer legitimen Anordnung, die Sie aus der Verletzung des Gesetzes zum Schutz ausgeführt. Ich möchte Sie zu verstehen, dass ich in dieser Bank seit 17 Jahren gearbeitet, und ich in der Lage, alle gesetzlichen Dokumente zu sichern, damit Sie diesen Fonds erben Bitte senden Sie mir der folgende: Wie haben wir einige Tage, um es zu durchlaufen, das ist sehr URGENT PLEASE. 1. Vollständiger Name 2. Ihre Direkt Mobile Number 3. Du bist Kontakt Adresse 4. Geburtsdatum Nachdem durch eine methodische Suche gegangen, entschied ich mich, Sie zu kontaktieren hoffen, dass Sie das Angebot interessant finden. Bitte auf Ihrer Bestätigung dieser Nachricht und geben Sie Ihr Interesse werde ich Ihnen weitere Informationen liefern. Bemühen, mich zu informieren Ihre Entscheidung so bald wie möglich. Freundliche Grüße, Mr.James Hartop -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: wishlist: make it possible to amend commit messages after push to remote
On Fri, Aug 07, 2015 at 06:50:52PM -0400, Jarkko Hietaniemi wrote: > Or another way to illustrate my idea: assume a create-once-no-delete > filesystem. > > echo 42 > the_answr.txt > > Oh, darn it... > > ln -s the_answr.txt the_answer.txt > > Now both names still point to the content "42\n". The first SHA > would be over ["42\n", "the_answr.txt"] and the second SHA over > ["42\n", "the_answer.txt"]. But you can't do that on a create-once filesystem; your symlink overwrites "the_answr.txt", which already exists. Obviously that is a technicality in the definition of "create-once", but if we take this as an analogy for a content-addressable object store, it is true. :) The name "answr.txt" in your case is really a sha1 "1234abcd", and we cannot create an object of that name that has anything _but_ the specific matching content in it. Your options are then: - add a level of indirection; when we look up 1234abcd, show some other object instead (even though its content does not match 1234abcd). We have this already; it is the git-replace mechanism. - during certain operations (e.g., showing the log), use 1234abcd as an index into another data store. We have this, too: git-notes. I think I saw the objection elsewhere in the thread that these are not seamless to use. That is certainly true. Partially this is inherent (the client has to understand the extra object, and know when you want that object versus the original). But git could also improve its handling of these things. For instance, we do not fetch notes from a remote by default. The big problems is that the refs/remotes hierarchy is set really set up only to hold branches, so we do not know where to put them. There was a discussion around the v1.8.0 time-frame about improving this[1], but it was never completed. That might be a productive direction if anyone is really interested in this topic. -Peff [1] I didn't re-read the old thread, but glancing through, this looks like a reasonable jumping-off point for reading: http://thread.gmane.org/gmane.comp.version-control.git/165799/focus=165885 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] sha1_file.c: rename move_temp_to_file() to finalize_temp_file()
On Fri, Aug 07, 2015 at 05:24:29PM -0700, Junio C Hamano wrote:
> Since 5a688fe4 ("core.sharedrepository = 0mode" should set, not
> loosen, 2009-03-25), we kept reminding ourselves:
>
> NEEDSWORK: this should be renamed to finalize_temp_file() as
> "moving" is only a part of what it does, when no patch between
> master to pu changes the call sites of this function.
>
> without doing anything about it. Let's do so.
>
> The purpose of this function was not to move but to finalize. The
> detail of the primarily implementation of finalizing was to link the
> temporary file to its final name and then to unlink, which wasn't
> even "moving". The alternative implementation did "move" by calling
> rename(2), which is a fun tangent.
This is definitely a better name. But while we are touching the area, my
other pet peeve about this function is that it is really specific to
moving _object_ temp files around. It started life as static-local to
sha1-file.c, so not mentioning objects is OK, but when it became a
global, it's a bit confusing.
Maybe finalize_object_file() would be a better name?
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
how are you◆
good news samsung s6,280euro, imac, Nikon, Samsung, gultar, ipad S i te: poazzlo .com
Re: resolving a (possibly remote) branch HEAD to a hash
Junio C Hamano wrote: > per...@pluto.rain.com (Perry Hutchison) writes: > > > $ git rev-parse r5.0.1 > > r5.0.1 > > fatal: ambiguous argument 'r5.0.1': unknown revision or path not in the > > working tree. > > Use '--' to separate paths from revisions > > This is not because of ambiguity among refs. The message is telling > you: > > r5.0.1 is not interpretable as a revision, and it is not likely > to be interpretable as a path. If you meant to use it as a > revision, put '--' after it, if you meant to use it as a path, > put '--' before it. > ... > Now, admittably, if you say "git rev-parse r5.0.1 --" in this > situation, it is not likely that the corrected command line will > succeed. Yes, I tried that also. It didn't work. > ... we do not say "append 'refs/remotes//' for various > values of and see if such a ref exists" when resolving > an abbreviated refname 'master'. checkout appears to. If I enter "git checkout foo" and the local branch refs/heads/foo exists checkout will: 1. recognize that foo refers to a local branch 2. check out that local branch (refs/heads/foo). OTOH, if I enter "git checkout foo" and there is no foo in refs/heads or refs/tags and refs/remotes/origin/foo _does_ exist checkout will: 1. recognize that foo refers to a remote branch 2. set up a local branch (refs/heads/foo) that tracks the remote branch (refs/remotes/origin/foo) 3. check out the (now) local branch (refs/heads/foo). However I don't think the command I want is spelled "checkout" because, in either case, I want just step 1 -- with the result being returned as the HEAD commit id of the selected branch. I don't want the whole checkout experience, just the identification of the commit that is the HEAD of the specified branch, which may be either local (in refs/heads) or remote (in refs/heads/origin). It is certainly possible that the command I am looking for is not spelled "rev-parse" either -- and it does not matter whether it is considered plumbing or porcelain. It just needs to produce the correct result, for either the local or the remote case. (In the situation at hand I happen to know that the name will always refer to a branch, not a tag, so a solution that looks only in refs/heads and refs/remotes -- and not in refs/tags -- would be fine. A solution that, like checkout, also looks in refs/tags would also be OK.) -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 03/11] ref-filter: implement an `align` atom
On Fri, Aug 7, 2015 at 9:34 AM, Eric Sunshine wrote: > On Wed, Aug 5, 2015 at 2:54 PM, Karthik Nayak wrote: >> Implement an `align` atom which will act as a modifier atom and align >> any string with or without an %(atom) appearing before a %(end) atom >> to the right, left or middle. >> >> It is followed by `:,`, where the `` is >> either left, right or middle and `` is the total length >> of the padding to be performed. If the atom length is more than the >> padding length then no padding is performed. e.g. to pad a succeeding >> atom to the middle with a total padding size of 40 we can do a >> --format="%(align:middle,40).." >> >> Add documentation and tests for the same. > > I forgot to mention in my earlier review of this patch that you should > explain in the commit message, and probably the documentation, this > this implementation (assuming I'm understanding the code) does not > correctly support nested %(foo)...%(end) constructs, where %(foo) > might be %(if:), %(truncate:), %(cut:), or even a nested %(align:), or > some as yet unimagined modifier. Supporting nesting of these > constructs will require pushing the formatting states onto a stack (or > invoking the parser recursively). > >> Signed-off-by: Karthik Nayak Good point, I have been working on this parallely and it works for now, I'll send that with the %(if) and %(end) feature. But for now, it should be documented and added in the commit message. Using a linked list of sorts where whenever a new modifier atom is encountered a new state is created, and once %(end) is encountered we can pop that state into the previous state. -- Regards, Karthik Nayak -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
