Skip to content

catch duplicate path names#3

Merged
adamnovak merged 1 commit into
masterfrom
dup-names
Jun 22, 2026
Merged

catch duplicate path names#3
adamnovak merged 1 commit into
masterfrom
dup-names

Conversation

@faithokamoto

Copy link
Copy Markdown

Was seeing if I could put in an easy fix for vgteam/vg#547 and it turns out the answer is yes, in that it requires few lines of code, and also no, in that I have no earthly idea how to properly make an update to this sub-submodule without causing unicorns to explode within the bowels of the interwebs. This extra check is part of what's needed.

The other thing that's needed is that when we add contigs (right after this line) we should have

                if (reference_for.count(kv.first)) {
                    #pragma omp critical (cerr)
                    cerr << "error:[vg::Constructor] Contig " << kv.first << " appears multiple times" << endl;
                    exit(1);
                }

but that only handles when there is a duplication between files, and this fastahack PR is needed for a duplication within a file.

@adamnovak once you're back, could you point me for how to do this sort of layered submodule update? The issue is ten years old so no rush.

@adamnovak

Copy link
Copy Markdown
Member

@faithokamoto Usually what I do is:

  1. Make a commit in the submodule and push it up to any fork of that submodule on Github.
  2. Make a commit in the parent project where I have git add-ed the submodule with the commit from step 1 checked out inside it.
  3. Push up the parent project commit to Github somewhere.
  4. Make two pull requests, one in the parent project and one in the submodule. Make sure one mentions the other, so we can tell they're related.

Because of the way Github works, Git can pull a particular commit of a submodule that's referenced in the parent project even when it hasn't been merged into the particular Github fork of the child project that the submodule reference points at.

This used to work great, but the last couple times I tried it, I got failures from the vg docs CI job, like https://ucsc-ci.com/vgteam/vg/-/jobs/110735, where that somehow wasn't working for whatever Git operation we're doing there. So either we need to fix that CI job, or we need to merge the child project PR before making the parent project PR (or at least rerun that job after the child project PR merges).

@adamnovak adamnovak merged commit 5b89a73 into master Jun 22, 2026
@adamnovak

Copy link
Copy Markdown
Member

@faithokamoto faithokamoto deleted the dup-names branch June 29, 2026 15:37
@faithokamoto

Copy link
Copy Markdown
Author

I am lost again. I cannot figure out how to push my branch to GitHub. For one, I can't figure out where the 7ca5c2036da5952ff1f262dedd26daafa2a1aeaf commit lives. It doesn't belong to the main vcflib https://github.com/vcflib/vcflib, nor can I find it in our vgteam vcflib https://github.com/vgteam/vcflib, and if I try to push to that vgteam vcflib it seems my 7ca5c2036da5952ff1f262dedd26daafa2a1aeaf-tied branch is wildly out of sync so that's definitely not how it works. Where can I put a GitHub PR to update the vcflib??

@adamnovak

Copy link
Copy Markdown
Member

It looks like that commit is on the update-fastahack branch of the vgteam/vcflib Github project: https://github.com/vgteam/vcflib/tree/update-fastahack. I figured this out by noticing I made it a year ago and looking at the list of branches in https://github.com/vgteam/vcflib/branches where (for me) it popped up under "My Branches". I think if you add the vgteam/vcflib project as a remote in the submodule (git remote add vgteam git@github.com:vgteam/vcflib.git) and then fetch from it (git fetch vgteam), then in the Git log for the commit (git log 7ca5c2036da5952ff1f262dedd26daafa2a1aeaf) it will tell you that there's a remote-tracking branch branch (vgteam/update-fastahack in red) that points to the commit.

I've opened a PR for the missing commit to put it in vgteam/vcflib's master branch. I should have done this when I set up vg to use it, but I didn't. I'll go ahead and merge that. (The CI we have from the upstream project seems to out of date to work, which is another reason we should maybe merge up their changes soon.)

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.

2 participants