Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

git: update dulwich to 0.20.22 #6049

Merged
merged 2 commits into from
May 24, 2021
Merged

Conversation

pmrowla
Copy link
Contributor

@pmrowla pmrowla commented May 24, 2021

Thank you for the contribution - we'll try to review it as soon as possible. πŸ™

@pmrowla pmrowla added the bugfix fixes bug label May 24, 2021
@pmrowla pmrowla self-assigned this May 24, 2021
@pmrowla pmrowla added the performance improvement over resource / time consuming tasks label May 24, 2021
@@ -10,6 +10,8 @@
os.environ["DVC_TEST"] = "true"
# Ensure progress output even when not outputting to raw sys.stderr console
os.environ["DVC_IGNORE_ISATTY"] = "true"
# Disable system git config
os.environ["GIT_CONFIG_NOSYSTEM"] = "1"
Copy link
Contributor Author

@pmrowla pmrowla May 24, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dulwich now reads the system-wide default config, which means it will read core.autocrlf=true for typical windows Git environments. This should be disabled in CI, as DVC does not read Git configs, and will not do any autocrlf normalization when importing from Git.

Several of our tests git-commit files with CRLF's (usually via Windows echo which behaves differently than unix echo). If autocrlf=true, the original workspace file will have\r\n line endings, but the git-committed (and git-index) version will have nomalized \n line endings.

If we dvc import or dvc get that git committed file, DVC will read/download the git-committed version containing \n endings (even on Windows). This will break tests on Windows which do a direct comparison between the "original" workspace file and the imported one (since the line endings will not match).


These tests passed with dulwich<0.20.22 because dulwich was always ignoring the system git config (meaning we always tested in CI with autocrlf disabled).


It looks like the tests passed when we only used gitpython because the gitpython index.add function we used for scm.add may have been adding files directly into the git index without doing any CRLF normalization (meaning we were always ignoring the user's autocrlf setting entirely w/gitpython whether or not we were in the CI environment).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix fixes bug performance improvement over resource / time consuming tasks
Projects
None yet
1 participant