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

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 (aff328648d8519e085a975767ebc865868256e09)

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:

https://github.com/nf-core/tools/blob/cc1ab618692d14c2203311009a21a187c92c47ef/nf_core/lint/files_unchanged.py#L135-L138

https://github.com/nf-core/tools/blob/cc1ab618692d14c2203311009a21a187c92c47ef/nf_core/create.py#L500

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

System information

nf-core, version 2.5.dev0 (cc1ab618692d14c2203311009a21a187c92c47ef)

About this issue

Original URL
State: closed
Created 2 years ago
Comments: 16 (14 by maintainers)

Most upvoted comments

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).

SamStudio8 on Jul 28, 2022

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.

mirpedrol on Jul 28, 2022

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.

https://github.com/nf-core/tools/blob/cc1ab618692d14c2203311009a21a187c92c47ef/nf_core/lint/files_unchanged.py#L135-L138 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.

SamStudio8 on Jul 28, 2022

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.

drpatelh on Jul 27, 2022