stg branch can't contain slash

[LinboLen]

such as stg new feature/core has no error.

but stg refresh will cause error

[jpgrayson]

I have confirmed that stg new feature/core does Bad Things on at least v0.18 and head of master. However, what I observe is that $EDITOR is invoked, then after saving the message, two errors are reported, and then, despite the errors, the patch exists in the series.

$ stg new -m "Core feature" feature/core
fatal: path feature/core contains slash
stg new: git failed with code 128
$ stg series
> feature/core
$ stg delete feature/core
Deleted feature/core (empty)
fatal: path feature/core contains slash
stg delete: git failed with code 128

I also note that .git/patches/master/patches/feature/core/* exist, but this patch does not show up in the meta file in the master.stgit branch.

I will try to find some time to add some test cases and repair this issue. Thanks @LinBoLen for reporting this.

[TerraTech]

Any news of patching this for 0.19 release? We use slashed branches quite a bit, e.g.:

• devel/newFeature
• pr/newFeature
• bugfix/oldFeature

For the record, we have recently switched from guilt to stgit and expect to hit this slash problem very soon as 'refresh' is our most heavily used command.

[jpgrayson]

I've taken another quick look at what's going on. The gist:

When a new patch is created (stg new), there is metadata created in a variety of places:

1. .git/patches
2. .git/refs/patches
3. The master.stgit branch; specifically in the patches tree therein.

For the first two, the patch name having a slash does not seem to be a problem--a patch name such as feature/core simply creates that directory hierarchy under .git/patches and .git/refs/patches.

In master.stgit, however, there is a problem with programmatically composing the patches tree. It appears that the code just does not anticipate patch names with slashes and thus ends up trying to make a blob object with the name, for example, feature/core, instead of what (I think) is needed: a tree object named feature parenting a blob object named core.

Ultimately, both stg new and stg refresh experience a failure when feeding the invalid feature/core blob name to git mktree -z. The fatal: path feature/core contains slash error comes from git mktree which explicitly checks that the names of objects going into a tree do not have slashes.

I think the right path forward is to make stgit properly comprehend this kind of hierarchical patch name. Obviously a repair for this did not make it into the 0.19 release, but I hope to make this repair and get it into a release shortly thereafter.

N.B. it may seem weird that stgit keeps metadata in so many places. That is because stgit is currently in the middle of a very long transition between the "old" metadata strategy where metadata is kept in files within the .git directory and the "new" strategy where metadata is kept in special branches (e.g. master.stgit). The goal for stgit 1.0 is to get everything converted to the new metadata infrastructure and thus eliminate the old infrastructure.

[TerraTech]

@jpgrayson thank you for the insight, we look forward to seeing this resolved in a beneficial way.

[akshedu]

@jpgrayson is this still open? Can I work on it?

[jpgrayson]

@akshedu yes, this issue remains open. And help is welcome, but...

One of the core problems at play, as I mentioned in a previous comment, is that StGit still has two distinct metadata systems at play ("old" and "new"). And this bug is very tightly intertwined with the details of how StGit deals with patch metadata.

As of the last StGit release (v0.23), all StGit commands use the new metadata system's API. However, I have not yet pushed the change that fully eliminates the old metadata and upgrades the metadata format (stack format version 3 ==> 4).

I have a patch that does the above. But needs more refinement. The stakes are fairly high, since migrating to the new metadata format is a one-way street and thus getting the auto-upgrade wrong or any number of other details wrong will make using/developing StGit from its git repo painful. Finishing the metadata updates and removing the compatibility code for the old metadata is at the top of my StGit list, but I nonetheless do not have a good prediction on when I'll complete that work.

Which is not to dissuade you from working on this bug, but I believe it will be much more tractable to repair in the context of the new metadata system and not in the context of both old and new systems.

[jpgrayson]

Now that StGit has entirely eliminated the old/new metadata dichotomy, I have taken another look at this issue. There seem to be two separate asks here:

1. Allow slashes in the branch name.
2. Allow slashes in the patch name.

As things stand, branch names with slashes are supported without any known issues. Note there are some specific test cases for this in t0001-subdir-branches.sh.

As for slashes in patch names, currently patch names with slashes are explicitly disallowed. StGit is now much more careful to validate patch names, so the specific pathology identified in this issue, where StGit's internal state becomes corrupted, no longer occurs.

I have experimented with modifications that allow slashses in patch names. And I was able to get it working. But I'm not sold that having slashes in patch names is a net value-add for StGit. My concerns are:

• The code needed to accommodate slashy patch names increases the maintenance burden. Anywhere in StGit that needs to comprehend patch names now also has to comprehend more complicated patch name collision semantics. I.e. that foo collides with foo/bar even though their names are different.
• There may be edge cases not yet covered by tests
• Having both slashy branche names (which is normal in git) and slashy patch names may prove confusing to users.

So my inclination is to stick with disallowing slashy patch names, but I'd also like to hear counter arguments from someone who sees value in slashy patch names.

I'll leave this issue open a while longer in case anyone wants to chime in.

[smcameron]

I have hit this issue just now (accidentally used a slash in a patch name without meaning to).

Stacked GIT 0.19
git version 2.25.1
Python version 3.8.10 (default, Nov 26 2021, 20:14:08) 
[GCC 9.3.0]

$ stg series
+ add-Wextra-to-MYCFLAGS
+ mark-unused-params
> png-utils-fix-signed/unsigned-compares

$ stg delete 'png-utils-fix-signed/unsigned-compares'
Deleted png-utils-fix-signed/unsigned-compares (empty)
fatal: path png-utils-fix-signed/unsigned-compares contains slash
stg delete: git failed with code 128

Any hints on how I clean this up?

Edit: For any others that may come along here looking for hints about how to fix this, what I ended up doing was abandoning this branch. "stg show patch-name" would not work, however, "git log", and "git show hash" would work, so I was able to salvage the other patches from this branch that way, and import them with a series of "stg new blah; patch -p1 < output-of-git-show.patch; stg refresh" commands.

[jpgrayson]

Hi @smcameron. Sorry that you got hit by this bug--the corrupted stack state makes this one particularly nasty. Your approach for recovering was good.

Newer versions of StGit (at least since v0.22) protect from this issue by failing gracefully if an attempt is made to make a patch with a slash in its name.

However, and unfortunately, Debian and its derivatives (i.e. Ubuntu) remain stuck at StGit v0.19, which was released about 3 years ago. So a lot of people remain exposed to this issue (I'm guessing your system fits in this category). Thus far, I've had bad luck getting a response from the nominal Debian package maintainer regarding updating to a recent StGit release. Maybe time to try again...

[smcameron]

Yep, that's the boat I'm in, along with countless others, which was why I figured it was worth posting here, just to record some recovery steps.

[jpgrayson]

StGit is not going to allow patch names with slashes.

Branch names with slashes are and will continue to be supported by StGit.

As noted above, both StGit 1.x releases as well as the upcoming 2.0 release do a good job of validating patch names and rejecting those that violate the patch naming rules. So with any version of StGit released in the last several years, patch names with slashes will be rejected without causing any corruption to the stack state.