init.defaultBranch=dev is not compatible with nf_core.create.git_init_pipeline

[SamStudio8]

NOTE Updated report after diagnosis (see comment thread), original report appended to this description.

Description of the bug

Running nf-core lint or nf-core create (v2.5.dev0) when git's init.defaultBranch is set to dev will exit with a GitCommandError traceback.

Command used and terminal output

(Remember to change your git config back, or do this in a VM!)

• git config --global init.defaultBranch dev
• Execute nf-core create (or nf-core lint in an existing workflow)

GitCommandError: Cmd('git') failed due to: exit code(128)
  cmdline: git branch dev
  stderr: 'fatal: A branch named 'dev' already exists.'

System information

nf-core, version 2.5.dev0 (aff3286)

---

Description of the bug

Running nf-core lint (v2.5.dev0) on a workflow inside a git repository with an existing dev branch yields a GitCommandError. It appears the file_unchanged lint step leads to the attempted creation of a branch (see traceback below).

Command used and terminal output

• Clone a workflow that has an existing dev branch (or create one)
• Execute nf-core lint

GitCommandError: Cmd('git') failed due to: exit code(128)
  cmdline: git branch dev
  stderr: 'fatal: A branch named 'dev' already exists.'

Is it expected that the file_unchanged lint step leads to the attempted creation of a branch? See:

tools/nf_core/lint/files_unchanged.py

Lines 135 to 138 in cc1ab61

	create_obj = nf_core.create.PipelineCreate(
	None, None, None, outdir=test_pipeline_dir, template_yaml_path=template_yaml_path
	)
	create_obj.init_pipeline()

tools/nf_core/create.py

Line 500 in cc1ab61

repo.git.branch("dev")

I'm wondering if the CreatePipeline call should be passing no_git=True?

System information

nf-core, version 2.5.dev0 (cc1ab61)

[mirpedrol]

Hi @SamStudio8, I am not able to reproduce this error, could you provide an example of the workflow that generated it? Thank you!

[SamStudio8]

Hi @mirpedrol , thanks for taking a look. My steps to reproduce this below:

python -m virtualenv ~/venv/nfcore-1688
source ~/venv/nfcore-1688/bin/activate
python -m pip install git+https://github.com/nf-core/tools.git@cc1ab618692d14c2203311009a21a187c92c47ef
mkdir nfcore-1688
cd nfcore-1688
git clone [email protected]:nf-core/sarek.git
cd sarek
nf-core lint

Update: I'm able to reproduce this from today's dev HEAD at aff328648d8519e085a975767ebc865868256e09.

[mirpedrol]

I'm sorry but still can't reproduce, I get other linting errors but the command runs fine.

│ files_unchanged: .github/PULL_REQUEST_TEMPLATE.md does not match the template                     │
│ files_unchanged: .github/workflows/linting.yml does not match the template                        │
│ files_unchanged: assets/email_template.txt does not match the template                            │
│ actions_ci: '.github/workflows/ci.yml' does not check minimum NF version  

I am wondering if it's related to the installation? I am using these versions:
Python 3.9.7
pip 22.2
nf-core/tools version 2.5.dev0
Nextflow version 21.10.6 build 5660

[SamStudio8]

@mirpedrol Thanks for trying that out once more! As you were unable to reproduce my error, I tried another machine and found the command does indeed work fine as you had demonstrated. As I could still reliably reproduce the error on the first machine, I followed each step of the traceback to try and determine what was happening. I confirmed that as part of linting for files_changed, a temporary repository is created in a temporary directory, and this routine is only performed once. All good so far.

Upon inspecting the temporary directory itself, I see that the repository has been created, and has two branches: TEMPLATE and dev. But, how can there be a dev branch before the call to repo.git.branch("dev") succeeds you ask? Yes, it's my ~/.gitconfig!

[init]
    defaultBranch = dev

I've updated the title of this issue to correctly identify the real problem: "init.defaultBranch=dev is not compatible with nf_core.create.git_init_pipeline". Indeed, any part of nf-core that uses git_init_pipeline including the nf-core create command will fail with the GitCommandError I opened this issue with, when the default branch is set to dev.

I'm not sure to what degree nf-core should worry about this particular edge case (ie. it might be out of scope to change the dev branch to a temporary name, or try and change the defaultBranch entirely), but it is always nice to seal up cases that blow up with an unhelpful traceback with a nice error instead. I note it is straightforward to use gitpython to find the defaultBranch (example below). Perhaps nf-core could at least catch the case where the user's default branch is dev, cowardly refuse to run commands and exit with a better warning?

>>> git.config.GitConfigParser().get_value("init", "defaultBranch")
'dev'

Thanks for your patience on this one!

[drpatelh]

Perhaps nf-core could at least catch the case where the user's default branch is dev, cowardly refuse to run commands and exit with a better warning?

Thanks @SamStudio8 ! This is a good idea. Especially, given how long it has taken to finally figure this out. Be nice to catch this properly.

[cjw85]

dev is a pretty common name for a branch, particularly for anyone following a git-flow style development process. For temporary branches a name that's unlikely to clash with any existing branch should be used. Here's some random data to get you started: e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855 😄

[SamStudio8]

dev is a pretty common name for a branch, particularly for anyone following a git-flow style development process. For temporary branches a name that's unlikely to clash with any existing branch should be used. Here's some random data to get you started: e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855 😄

Sorry @cjw85 I have some files using that data already.

[drpatelh]

Yeah me too 👀

[ewels]

dev is a pretty common name for a branch, particularly for anyone following a git-flow style development process.

@cjw85 yes, but nf-core create is creating a brand new git repo from scratch. So unless you have init.defaultBranch in your git config it's basically impossible to have a conflict with the branch name.

This is the first time this has come up in ~4 years and 100s of people running the command, so it's fairly far out in edge-case territory 😅 (kudos! 🙇🏻 )

[cjw85]

I believe Sam is suggesting that nf-core lint has this issue independent of nf-core create.

[ewels]

Yup and I think it should at least fail gracefully in both cases 🙂 Just saying that I don't think that we need to change any behaviour. Not sure why lint would be failing with this 🤔 I'd be curious to see where that's coming from.

[SamStudio8]

I believe Sam is suggesting that nf-core lint has this issue independent of nf-core create.

I had thought initially that the error was related to the existence of a dev branch in the repository of the pipeline being linted, but as I have documented in my recent comment this is failing because the temporary branch can't have a dev branch added if one already exists when the repository is created.

Not sure why lint would be failing with this 🤔 I'd be curious to see where that's coming from.

I had the right code but the wrong root cause in my original issue above. The files_unchanged module of nf-core lint uses nf_core.create.PipelineCreate to create an nf-core pipeline with the latest template in a temporary directory (such that the files between the pipeline being linted and the latest template can be compared). But by default, it also initialises a git repository for the pipeline.

tools/nf_core/lint/files_unchanged.py

Lines 135 to 138 in cc1ab61

	create_obj = nf_core.create.PipelineCreate(
	None, None, None, outdir=test_pipeline_dir, template_yaml_path=template_yaml_path
	)
	create_obj.init_pipeline()

I think in the case of nf-core lint you could actually pass no_git=True to PipelineCreate here, and skip making a git repository entirely, as the linter does not seem to require a git repository to compare the template files (but I haven't tried this). This could at least allow linting without checking the defaultBranch, and does not preclude merging #1705.

[mirpedrol]

Thanks @SamStudio8 for all the testing and suggestions!
I am implementing a sanity check in #1705 to make the command fail if the default branch name coincides with one of the branches created by PipelineCreate.

[SamStudio8]

This is the first time this has come up in ~4 years and 100s of people running the command, so it's fairly far out in edge-case territory 😅 (kudos! 🙇🏻 )

For what it's worth, I wondered about this and discovered that init.defaultBranch was added to git in v2.28.0 (circa 2020). Many distros are still shipping with v2.25.0 (where the default branch is hard coded in the git source as master).

[ewels]

Ahh nice, thanks for digging into this @SamStudio8 - much appreciated!

[mirpedrol]

I am closing this issue since it is fixed by #1705, thanks!!