SCM: allow multiple DVC repos inside single SCM repo #3257

pared · 2020-01-30T15:03:41Z

❗ Have you followed the guidelines in the Contributing to DVC list?
📖 Check this box if this PR does not require documentation updates, or if it does and you have created a separate PR in dvc.org with such updates (or at least opened an issue about it in that repo). Please link below to your PR (or issue) in the dvc.org repo.
❌ Have you checked DeepSource, CodeClimate, and other sanity checks below? We consider their findings recommendatory and don't expect everything to be addressed. Please review them carefully and fix those that actually improve code or fix bugs.

Thank you for the contribution - we'll try to review it as soon as possible. 🙏

dvc/config.py

pared · 2020-02-03T10:38:18Z

@dmpetrov I noticed that in #2706 you mentioned that managing subrepos from "master" repo might be a viable use case. In this issue, I introduced DvcIgnore that would ignore subrepos when working from master repo. Do you think we should allow dvc pull and other "data sync" commands to work for subrepos?

dvc/command/init.py

dvc/repo/__init__.py

dvc/repo/init.py

tests/func/test_add.py

dvc/ignore.py

dvc/scm/__init__.py

skshetry · 2020-02-05T08:17:27Z

get and import won't work for the dvc subrepo, right? (as it's Git-repo based, not dvc-repo based, see: #3277).

dmpetrov

I've tested it a bit - looks good to me.
A couple comments are inline. The major one - if we need "subrepo" in config.

dvc/command/init.py

dvc/config.py

dmpetrov · 2020-02-10T19:22:15Z

Do you think we should allow dvc pull and other "data sync" commands to work for subrepos?

@pared No, commands should not go to nested repos.

PS: we can even think about not supporting nested repos if it simplifies implementation or docs.

dvc/repo/__init__.py

pared · 2020-02-12T10:32:41Z

@dmpetrov ok, so in the end some kind of flag will have to be introduced to config (take note of this discussion: #3257 (comment))

I think we should go with the second option I mentioned there, which is introducing no-scm config option when --no-scm is used. That will break backward compatibility for existing no-scm repos
but the fix will be relatively easy (writing no-scm=True in config). Basing on user activity on discord and in issues, I would not expect too much of such cases.
Having subrepo as default behavior makes more sense to me, since this subject is recurring.

efiop · 2020-02-14T11:15:33Z

@pared Also something to note about no-scm, is that it will require changing our tests, because base dvc fixture doesn't have git in it by default, it is init-ed with no_scm=True and then when git is applied on top.

dvc/scm/__init__.py

dvc/config.py

dvc/repo/init.py

jorgeorpinel

All good from my end. Just a question above ^ Thanks

efiop · 2020-02-24T17:08:12Z

@pared Looks good! How about docs though? Could you please create a doc PR before we merge this?

dvc/repo/__init__.py

pared · 2020-02-25T12:43:29Z

PR for docs: iterative/dvc.org#1022

jorgeorpinel · 2020-02-26T01:43:06Z

dvc/ignore.py

 class DvcIgnoreFilter(object):
    def __init__(self, tree):
        self.tree = tree
-        self.ignores = {DvcIgnoreDirs([".git", ".hg", ".dvc"])}
+        self.ignores = {
+            DvcIgnoreDirs([".git", ".hg", ".dvc"]),


.hg ? Do we support Mercurial somehow now??? Or is this legacy stuff? Thanks

We don't want to traverse through .git or .hg folders. People might use hg for code versioning and use --no-scm with DVC.

Hm... Why only do this for .hg? There's many many possible files or dirs the user may want to gitignore and they can just add it to .gitignore/ or .git/info/exclude

@jorgeorpinel add .hg to .gitignore? Maybe I'm missing something.

An alternative would be to add .hg to .dvcignore file, yes, but since we know about .hg, we are skipping it automatically.

add .hg to .gitignore?

Haha sorry I wasn't thinking right 😋

to add .hg to .dvcignore

Yeah, this! Since we don't support Mercurial in general why treat it specially? I'd let the user handle it, like he could want dvcignore any other files or directories for a bunch of reasons.

DVC has --no-scm, so they could be using any version control out there

I don't think this is the case @skshetry. It's just kind of a legacy option name AFAIK. Refer to #2901.

Do people really use .hg folders for their work? Unless they do (which they shouldn't)

You'd be surprised.
Why shouldn't they? Will the directory name police pul them over? 😛

I realized that there might be indeed a stronger motivation to include more defaults

@shcheklein I don't even see this. DVC ignores .git/ automatically because it integrates with Git. I don't see a reason to treat any other file/dir name specially.
Your questions about .gitignore vs. .dvcignore are interesting though; I think there may be some issues already about that.

In summary, I vote to remove the 7 characters we're talking about, but I realize this is probably a moot point since no one even uses Mercurial anymore haha.

@jorgeorpinel, I think something good thing to do is in middle. If people have a certain directories such as __pycache__ and venv or similar kinds of directories, it'd be good to make them aware that .dvcignore exists, at least a warning that traversing will be slow.

Agree about improving .dvcignore awareness. (I think the warning may exist BTW?)

For docs, I opened iterative/dvc.org/issues/1033.

at least a warning that traversing will be slow.

it's a really good idea. Please create an issue for this in the DVC core repo.

I still don't see a reason to include .hg though. To my mind if we follow this logic it should be done properly like you described for example.

Created #3436

weekly-digest bot mentioned this pull request Feb 2, 2020

Weekly Digest (26 January, 2020 - 2 February, 2020) #3272

Closed

efiop reviewed Feb 2, 2020

View reviewed changes

dvc/config.py Outdated Show resolved Hide resolved

dmpetrov mentioned this pull request Feb 3, 2020

Allow restriction dvc pull to targets #2706

Closed

pared force-pushed the 2349_many_dvc_roots branch from f65faf1 to 23ad958 Compare February 4, 2020 10:38

pared changed the title ~~[WIP] SCM: allow multiple DVC repos inside single SCM repo~~ SCM: allow multiple DVC repos inside single SCM repo Feb 4, 2020

pared marked this pull request as ready for review February 4, 2020 11:43