dvc: change dvcfile.dump to do synchronized dump by default (#4544)

The API of `Dvcfile.dump` has been changed to
`dump(stage, update_pipeline=True, update_lock=True)`,
which will make it to always dump by default to both files,
and provide flags to suppress one or the other.

Fixes #4235

dvc/dvcfile.py

@@ -175,7 +175,7 @@ class PipelineFile(FileMixin):
         return Lockfile(self.repo, os.path.splitext(self.path)[0] + ".lock")
 
     def dump(
-        self, stage, update_pipeline=False, no_lock=False, **kwargs
+        self, stage, update_pipeline=True, update_lock=True, **kwargs
     ):  # pylint: disable=arguments-differ
         """Dumps given stage appropriately in the dvcfile."""
         from dvc.stage import PipelineStage
@@ -187,7 +187,7 @@ class PipelineFile(FileMixin):
         if update_pipeline and not stage.is_data_source:
             self._dump_pipeline_file(stage)
 
-        if not no_lock:
+        if update_lock:
             self._dump_lockfile(stage)
 
     def _dump_lockfile(self, stage):

dvc/repo/commit.py

@@ -45,5 +45,5 @@ def commit(self, target, with_deps=False, recursive=False, force=False):
             stage.save()
         stage.commit()
 
-        Dvcfile(self, stage.path).dump(stage)
+        Dvcfile(self, stage.path).dump(stage, update_pipeline=False)
     return stages

dvc/repo/freeze.py

@@ -8,7 +8,7 @@ def _set(repo, target, frozen):
     path, name = parse_target(target)
     stage = repo.get_stage(path, name)
     stage.frozen = frozen
-    stage.dvcfile.dump(stage, update_pipeline=True)
+    stage.dvcfile.dump(stage, update_lock=False)
 
     return stage
 

dvc/repo/plots/__init__.py

@@ -138,7 +138,7 @@ class Plots:
         out.verify_metric()
 
         dvcfile = Dvcfile(self.repo, out.stage.path)
-        dvcfile.dump(out.stage, update_pipeline=True, no_lock=True)
+        dvcfile.dump(out.stage, update_lock=False)
 
 
 def _collect_plots(repo, targets=None, rev=None):

dvc/repo/reproduce.py

@@ -25,7 +25,7 @@ def _reproduce_stage(stage, **kwargs):
         from ..dvcfile import Dvcfile
 
         dvcfile = Dvcfile(stage.repo, stage.path)
-        dvcfile.dump(stage)
+        dvcfile.dump(stage, update_pipeline=False)
 
     return [stage]
 

dvc/repo/run.py

@@ -134,5 +134,5 @@ def run(self, fname=None, no_exec=False, single_stage=False, **kwargs):
             run_cache=kwargs.get("run_cache", True),
         )
 
-    dvcfile.dump(stage, update_pipeline=True, no_lock=no_exec)
+    dvcfile.dump(stage, update_lock=not no_exec)
     return stage

tests/func/test_dvcfile.py

@@ -242,7 +242,7 @@ def test_remove_stage_on_lockfile_format_error(tmp_dir, dvc, run_copy):
         dvc_file.remove_stage(stage)
 
     lock_file.remove()
-    dvc_file.dump(stage)
+    dvc_file.dump(stage, update_pipeline=False)
 
     dump_yaml(dvc_file.relpath, data)
     with pytest.raises(StageFileFormatError):
@@ -312,7 +312,7 @@ def test_dvcfile_dump_preserves_meta(tmp_dir, dvc, run_copy):
     data["stages"]["run_copy"]["meta"] = metadata
     dump_yaml(dvcfile.path, data)
 
-    dvcfile.dump(stage, update_pipeline=True)
+    dvcfile.dump(stage)
     assert dvcfile._load()[0] == data
     assert dvcfile._load()[0]["stages"]["run_copy"]["meta"] == metadata
 
@@ -332,5 +332,5 @@ def test_dvcfile_dump_preserves_comments(tmp_dir, dvc):
     stage.outs[0].use_cache = False
     dvcfile = stage.dvcfile
 
-    dvcfile.dump(stage, update_pipeline=True)
+    dvcfile.dump(stage)
     assert dvcfile._load()[1] == (text + ":\n\tcache: false\n".expandtabs())

tests/unit/test_dvcfile.py

@@ -13,6 +13,7 @@ from dvc.stage.exceptions import (
     StageFileFormatError,
     StageFileIsNotDvcFileError,
 )
+from dvc.utils.fs import remove
 from dvc.utils.serialize import dump_yaml
 
 
@@ -67,15 +68,18 @@ def test_dump_stage(tmp_dir, dvc):
     )
     dvcfile = Dvcfile(dvc, "dvc.yaml")
 
-    dvcfile.dump(stage, no_lock=True)
+    dvcfile.dump(stage, update_lock=False, update_pipeline=False)
     assert not (tmp_dir / PIPELINE_FILE).exists()
     assert not (tmp_dir / PIPELINE_LOCK).exists()
 
-    dvcfile.dump(stage, no_lock=False)
+    dvcfile.dump(stage, update_pipeline=False)
     assert not (tmp_dir / PIPELINE_FILE).exists()
+    assert (tmp_dir / PIPELINE_LOCK).exists()
     assert dvcfile._lockfile.load()
 
-    dvcfile.dump(stage, update_pipeline=True, no_lock=False)
+    remove(tmp_dir / PIPELINE_LOCK)
+
+    dvcfile.dump(stage)
     assert (tmp_dir / PIPELINE_FILE).exists()
     assert (tmp_dir / PIPELINE_LOCK).exists()
     assert list(dvcfile.stages.values()) == [stage]