[Notes] [Git][BuildStream/buildstream][gokcennurlu/remote_url_override

Gökçen Nurlu pushed to branch gokcennurlu/remote_url_override_push_error at BuildStream / buildstream

Commits:

1b24148f

by Chandan Singh at 2018-12-03T09:46:13Z

Ensure `--deps=none` option works for `bst checkout`

Currently, `bst checkout --deps none` command always produces empty
output. Fix this issue and add regression test for the same.

- element_enums.py: Add Scope.NONE.

- element.py: Ensure Scope.NONE works correctly in addition to
  Scope.RUN/Scope.ALL in Element.dependencies() and Element.search().

- tests/frontend/buildcheckout.py: Fix tests for `--deps none`.

Fixes #670.

8ac1e757

by Chandan Singh at 2018-12-03T09:46:14Z

Add `--deps build` option to `bst checkout`

As discussed in
https://mail.gnome.org/archives/buildstream-list/2018-September/msg00064.html,
add `--deps build` option to `bst checkout`. This will allow users to
checkout the all build dependencies of a given element using a single
command.

- _frontend/cli.py: Add `--deps build` option for `bst checkout`.

- element.py: Support `deps='build'` in Element._prepare_sandbox().

- tests/frontend/buildcheckout.py: Ensure `--deps build` works as
  expected.

ac60f47a

by Benjamin Schubert at 2018-12-03T09:46:14Z

Refactor and simplify _prepare_sandbox for elements

Before we would have a intricate logics with multiple arguments
that might get ignored.

This simplifies the design and introduces a bool `shell` instead
of having two different variables concerned about scope

b1430fb5

by Tristan Van Berkom at 2018-12-03T11:01:54Z

Merge branch 'chandan/bst-checkout-build' into 'master'

Add `--deps build` option to `bst checkout`

Closes #670

See merge request BuildStream/buildstream!819

9022a778

by Javier Jardón at 2018-12-03T11:14:55Z

buildstream/plugins/sources/git.py: warn about checkout by default

See #783

4462892e

by Tristan Van Berkom at 2018-12-03T11:45:07Z

Merge branch 'jjardon/doc_git_submodules' into 'master'

git: warn about checkout submodules  by default

See merge request BuildStream/buildstream!983

4ae2bf56

by Gökçen Nurlu at 2018-12-03T15:34:03Z

Set ArtifactCache for push/pull correctly when `--remote` is used

This change makes `ArtifactCache.setup_remotes()` use a boolean parameter, set
by `Stream.push()` or `Stream.pull()`, to configure the overriden `remote_url`
correctly, instead of for `push` by default.

This fixes #789.

4b772a9b

by Gökçen Nurlu at 2018-12-03T15:34:03Z

Fix type of artifact_remote_url in _load() docs

1f77d9f3

by Gökçen Nurlu at 2018-12-03T15:34:03Z

Refactor setup_remotes() to remove --remote overriding logic

This change simplifies the responsibilities of `setup_remotes()` by extracting
"getting the default remotes" logic to its own function and moving "--remote"
related logic to _load(). The latter, especially, helps to remove the need of
passing the "remote_url" string and "can_push?" boolean to `setup_remotes()`.

7 changed files:

buildstream/_artifactcache/artifactcache.py
buildstream/_frontend/cli.py
buildstream/_stream.py
buildstream/element.py
buildstream/plugins/sources/git.py
buildstream/types.py
tests/frontend/buildcheckout.py

Changes:

buildstream/_artifactcache/artifactcache.py

@@ -110,36 +110,42 @@ class ArtifactCache():
          # assume project and element names are not allowed to contain slashes
          return '{0}/{1}/{2}'.format(project.name, element_name, key)
 +    # get_remotes_from_projects()
 +    #
 +    # Generates list artifact caches based on project configuration
 +    #
 +    # Returns:
 +    #    (list of (list of ArtifactCacheSpec, Project)): Configurations each are
 +    #        ready to be consumed by `self._set_remotes()`
 +    #
 +    # This requires that all of the projects which are to be processed in the session
 +    # have already been loaded and are observable in the Context.
 +    #
 +    def get_remotes_from_projects(self):
 +        return [
 +            (_configured_remote_artifact_cache_specs(self.context, prj), prj)
 +            for prj in self.context.get_projects()
 +        ]
++
      # setup_remotes():
+     #
      # Sets up which remotes to use
+     #
      # Args:
 -    #    use_config (bool): Whether to use project configuration
 -    #    remote_url (str): Remote artifact cache URL
 +    #    remotes (list of (list of ArtifactCacheSpec, Project)): Configurations each are
 +    #        ready to be consumed by `self._set_remotes()`
+     #
      # This requires that all of the projects which are to be processed in the session
      # have already been loaded and are observable in the Context.
+     #
 -    def setup_remotes(self, *, use_config=False, remote_url=None):
+-
 +    def setup_remotes(self, *, remotes):
          # Ensure we do not double-initialise since this can be expensive
          assert not self._remotes_setup
          self._remotes_setup = True
 -        # Initialize remote artifact caches. We allow the commandline to override
 -        # the user config in some cases (for example `bst push --remote=...`).
 -        has_remote_caches = False
 -        if remote_url:
 -            self._set_remotes([ArtifactCacheSpec(remote_url, push=True)])
 -            has_remote_caches = True
 -        if use_config:
 -            for project in self.context.get_projects():
 -                artifact_caches = _configured_remote_artifact_cache_specs(self.context, project)
 -                if artifact_caches:  # artifact_caches is a list of ArtifactCacheSpec instances
 -                    self._set_remotes(artifact_caches, project=project)
 -                    has_remote_caches = True
 -        if has_remote_caches:
 +        if remotes:
 +            for caches, project in remotes:
 +                self._set_remotes(caches, project=project)
              self._initialize_remotes()
      # specs_from_config_node()

buildstream/_frontend/cli.py

@@ -640,7 +640,7 @@ def shell(app, element, sysroot, mount, isolate, build_, command):
  @click.option('--force', '-f', default=False, is_flag=True,
                help="Allow files to be overwritten")
  @click.option('--deps', '-d', default='run',
 -              type=click.Choice(['run', 'none']),
 +              type=click.Choice(['run', 'build', 'none']),
                help='The dependencies to checkout (default: run)')
  @click.option('--integrate/--no-integrate', default=True, is_flag=True,
                help="Whether to run integration commands")
@@ -657,16 +657,24 @@ def shell(app, element, sysroot, mount, isolate, build_, command):
  def checkout(app, element, location, force, deps, integrate, hardlinks, tar):
      """Checkout a built artifact to the specified location
      """
 +    from ..element import Scope
      if hardlinks and tar:
          click.echo("ERROR: options --hardlinks and --tar conflict", err=True)
          sys.exit(-1)
 +    if deps == "run":
 +        scope = Scope.RUN
 +    elif deps == "build":
 +        scope = Scope.BUILD
 +    elif deps == "none":
 +        scope = Scope.NONE
++
      with app.initialized():
          app.stream.checkout(element,
                              location=location,
                              force=force,
 -                            deps=deps,
 +                            scope=scope,
                              integrate=integrate,
                              hardlinks=hardlinks,
                              tar=tar)

buildstream/_stream.py

@@ -28,6 +28,7 @@ import tarfile
  from contextlib import contextmanager
  from tempfile import TemporaryDirectory
 +from ._artifactcache import ArtifactCacheSpec
  from ._exceptions import StreamError, ImplError, BstError, set_last_task_error
  from ._message import Message, MessageType
  from ._scheduler import Scheduler, SchedStatus, TrackQueue, FetchQueue, BuildQueue, PullQueue, PushQueue
@@ -305,6 +306,7 @@ class Stream():
                                   selection=selection,
                                   use_artifact_config=use_config,
                                   artifact_remote_url=remote,
 +                                 artifact_remote_can_push=False,
                                   fetch_subprojects=True)
          if not self._artifacts.has_fetch_remotes():
@@ -343,6 +345,7 @@ class Stream():
                                   selection=selection,
                                   use_artifact_config=use_config,
                                   artifact_remote_url=remote,
 +                                 artifact_remote_can_push=True,
                                   fetch_subprojects=True)
          if not self._artifacts.has_push_remotes():
@@ -370,7 +373,7 @@ class Stream():
      #    target (str): Target to checkout
      #    location (str): Location to checkout the artifact to
      #    force (bool): Whether files can be overwritten if necessary
 -    #    deps (str): The dependencies to checkout
 +    #    scope (str): The scope of dependencies to checkout
      #    integrate (bool): Whether to run integration commands
      #    hardlinks (bool): Whether checking out files hardlinked to
      #                      their artifacts is acceptable
@@ -383,7 +386,7 @@ class Stream():
      def checkout(self, target, *,
                   location=None,
                   force=False,
 -                 deps='run',
 +                 scope=Scope.RUN,
                   integrate=True,
                   hardlinks=False,
                   tar=False):
@@ -396,7 +399,7 @@ class Stream():
          # Stage deps into a temporary sandbox first
          try:
 -            with target._prepare_sandbox(Scope.RUN, None, deps=deps,
 +            with target._prepare_sandbox(scope=scope, directory=None,
                                           integrate=integrate) as sandbox:
                  # Copy or move the sandbox to the target directory
@@ -922,7 +925,8 @@ class Stream():
      #    track_except_targets (list of str): Specified targets to except from fetching
      #    track_cross_junctions (bool): Whether tracking should cross junction boundaries
      #    use_artifact_config (bool): Whether to initialize artifacts with the config
 -    #    artifact_remote_url (bool): A remote url for initializing the artifacts
 +    #    artifact_remote_url (str): A remote url for initializing the artifacts
 +    #    artifact_remote_can_push (bool): Whether `artifact_remote_url` can be used to push
      #    fetch_subprojects (bool): Whether to fetch subprojects while loading
+     #
      # Returns:
@@ -937,6 +941,7 @@ class Stream():
                track_cross_junctions=False,
                use_artifact_config=False,
                artifact_remote_url=None,
 +              artifact_remote_can_push=False,
                fetch_subprojects=False,
                dynamic_plan=False):
@@ -1000,12 +1005,20 @@ class Stream():
              self._pipeline.resolve_elements(track_selected)
              return [], track_selected
 -        # ArtifactCache.setup_remotes expects all projects to be fully loaded
 -        for project in self._context.get_projects():
 -            project.ensure_fully_loaded()
+-
 +        if use_artifact_config:
 +            # ArtifactCache.get_remotes_from_projects expects all projects to be
 +            # fully loaded
 +            for project in self._context.get_projects():
 +                project.ensure_fully_loaded()
 +            remotes = self._artifacts.get_remotes_from_projects()
 +        else:
 +            # Build the ArtifactCacheSpec instance based on `--remote`
 +            remotes = [(
 +                [ArtifactCacheSpec(artifact_remote_url, push=artifact_remote_can_push)],
 +                None
 +            )]
          # Connect to remote caches, this needs to be done before resolving element state
 -        self._artifacts.setup_remotes(use_config=use_artifact_config, remote_url=artifact_remote_url)
 +        self._artifacts.setup_remotes(remotes=remotes)
          # Now move on to loading primary selection.
+         #

buildstream/element.py

@@ -438,7 +438,7 @@ class Element(Plugin):
                                                  visited=visited, recursed=True)
          # Yeild self only at the end, after anything needed has been traversed
 -        if should_yield and (recurse or recursed) and (scope in (Scope.ALL, Scope.RUN)):
 +        if should_yield and (recurse or recursed) and scope != Scope.BUILD:
              yield self
      def search(self, scope, name):
@@ -1339,7 +1339,7 @@ class Element(Plugin):
      # is used to stage things by the `bst checkout` codepath
+     #
      @contextmanager
 -    def _prepare_sandbox(self, scope, directory, deps='run', integrate=True):
 +    def _prepare_sandbox(self, scope, directory, shell=False, integrate=True):
          # bst shell and bst checkout require a local sandbox.
          bare_directory = True if directory else False
          with self.__sandbox(directory, config=self.__sandbox_config, allow_remote=False,
@@ -1350,20 +1350,19 @@ class Element(Plugin):
              # Stage something if we need it
              if not directory:
 -                if scope == Scope.BUILD:
 +                if shell and scope == Scope.BUILD:
                      self.stage(sandbox)
 -                elif scope == Scope.RUN:
 +                else:
                      # Stage deps in the sandbox root
 -                    if deps == 'run':
 -                        with self.timed_activity("Staging dependencies", silent_nested=True):
 -                            self.stage_dependency_artifacts(sandbox, scope)
 +                    with self.timed_activity("Staging dependencies", silent_nested=True):
 +                        self.stage_dependency_artifacts(sandbox, scope)
 -                        # Run any integration commands provided by the dependencies
 -                        # once they are all staged and ready
 -                        if integrate:
 -                            with self.timed_activity("Integrating sandbox"):
 -                                for dep in self.dependencies(scope):
 -                                    dep.integrate(sandbox)
 +                    # Run any integration commands provided by the dependencies
 +                    # once they are all staged and ready
 +                    if integrate:
 +                        with self.timed_activity("Integrating sandbox"):
 +                            for dep in self.dependencies(scope):
 +                                dep.integrate(sandbox)
              yield sandbox
@@ -1858,7 +1857,7 @@ class Element(Plugin):
      # If directory is not specified, one will be staged using scope
      def _shell(self, scope=None, directory=None, *, mounts=None, isolate=False, prompt=None, command=None):
 -        with self._prepare_sandbox(scope, directory) as sandbox:
 +        with self._prepare_sandbox(scope, directory, shell=True) as sandbox:
              environment = self.get_environment()
              environment = copy.copy(environment)
              flags = SandboxFlags.INTERACTIVE | SandboxFlags.ROOT_READ_ONLY

buildstream/plugins/sources/git.py

@@ -25,6 +25,11 @@ git - stage files from a git repository
    * git
 +.. attention::
++
 +    Note that this plugin **will checkout git submodules by default**; even if
 +    they are not specified in the `.bst` file.
++
  **Usage:**
  .. code:: yaml

buildstream/types.py

@@ -48,6 +48,12 @@ class Scope(Enum):
      itself.
      """
 +    NONE = 4
 +    """Just the element itself, no dependencies.
++
 +    *Since: 1.4*
 +    """
++
  class Consistency():
      """Defines the various consistency states of a :class:`.Source`.

tests/frontend/buildcheckout.py

@@ -86,13 +86,14 @@ def test_build_invalid_suffix_dep(datafiles, cli, strict, hardlinks):
  @pytest.mark.datafiles(DATA_DIR)
 -@pytest.mark.parametrize("deps", [("run"), ("none")])
 +@pytest.mark.parametrize("deps", [("run"), ("none"), ("build")])
  def test_build_checkout_deps(datafiles, cli, deps):
      project = os.path.join(datafiles.dirname, datafiles.basename)
      checkout = os.path.join(cli.directory, 'checkout')
 +    element_name = "checkout-deps.bst"
      # First build it
 -    result = cli.run(project=project, args=['build', 'target.bst'])
 +    result = cli.run(project=project, args=['build', element_name])
      result.assert_success()
      # Assert that after a successful build, the builddir is empty
@@ -101,20 +102,25 @@ def test_build_checkout_deps(datafiles, cli, deps):
      assert not os.listdir(builddir)
      # Now check it out
 -    result = cli.run(project=project, args=['checkout', 'target.bst', '--deps', deps, checkout])
 +    result = cli.run(project=project, args=['checkout', element_name, '--deps', deps, checkout])
      result.assert_success()
 -    # Check that the executable hello file is found in the checkout
 -    filename = os.path.join(checkout, 'usr', 'bin', 'hello')
 +    # Verify output of this element
 +    filename = os.path.join(checkout, 'etc', 'buildstream', 'config')
 +    if deps == "build":
 +        assert not os.path.exists(filename)
 +    else:
 +        assert os.path.exists(filename)
 -    if deps == "run":
 +    # Verify output of this element's build dependencies
 +    filename = os.path.join(checkout, 'usr', 'include', 'pony.h')
 +    if deps == "build":
          assert os.path.exists(filename)
      else:
          assert not os.path.exists(filename)
 -    # Check that the executable hello file is found in the checkout
 -    filename = os.path.join(checkout, 'usr', 'include', 'pony.h')
+-
 +    # Verify output of this element's runtime dependencies
 +    filename = os.path.join(checkout, 'usr', 'bin', 'hello')
      if deps == "run":
          assert os.path.exists(filename)
      else:

[Notes] [Git][BuildStream/buildstream][gokcennurlu/remote_url_override_push_error] 9 commits: Ensure `--deps=none` option works for `bst checkout`

Gökçen Nurlu pushed to branch gokcennurlu/remote_url_override_push_error at BuildStream / buildstream

Commits:

7 changed files:

Changes: