[Notes] [Git][BuildStream/buildstream][aevri/are_you

Angelos Evripiotis pushed to branch aevri/are_you_sure at BuildStream / buildstream

Commits:

3590ca8c

by Abderrahim Kitouni at 2019-01-28T14:59:19Z

requirements/requirements.in: require protobuf >= 3.6

This is needed since 0f2bc3754

39b952dc

by Abderrahim Kitouni at 2019-01-28T15:19:04Z

requirements/requirements.in: require Click >= 7.0

This is needed since 629a6e524, and was lost in the conversion to requirements.in

80b36d0c

by Javier Jardón at 2019-01-28T17:54:52Z

Merge branch 'abderrahim/protobuf-version' into 'master'

requirements/requirements.in: update minimum versions

Closes #884

See merge request BuildStream/buildstream!1114

a1ab48da

by Valentin David at 2019-01-28T21:30:26Z

Fix crash when spawned job completes very fast

Job can complete before we return from `Job.span()` to
`Scheduler._spawn_job()`, so that `_active_jobs` would not yet contain
the job.

This would print a stack on the console and try to run a second time
the job which can have unexpected effects.

In order to reproduce the issue, in
`buildstream/_scheduler/jobs/job.py`, in `Job.spawn`,
add a call to `time.sleep()` right before call to
`asyncio.get_child_watcher()`.

This fixes issue #857.

2fcb4491

by Jürg Billeter at 2019-01-28T22:36:22Z

Merge branch 'valentindavid/crash_in_scheduler_857' into 'master'

Fix crash when spawned job completes very fast

Closes #857

See merge request BuildStream/buildstream!1095

1c05a092

by Valentin David at 2019-01-29T05:58:17Z

Fix type of error codes in CAS server

Fixes #882.

785da59c

by Jürg Billeter at 2019-01-29T06:49:10Z

Merge branch 'valentindavid/wrong_type_in_status_code' into 'master'

Fix type of error codes in CAS server

Closes #882

See merge request BuildStream/buildstream!1099

ddef91ea

by Valentin David at 2019-01-29T07:23:35Z

Make sure testing cache directory exists

Fixes #873

6a4c8611

by Jürg Billeter at 2019-01-29T08:18:45Z

Merge branch 'valentindavid/make_cache_dir' into 'master'

Make sure testing cache directory exists

Closes #873

See merge request BuildStream/buildstream!1092

86c8e414

by Angelos Evripiotis at 2019-01-29T10:39:29Z

BREAK:remove unconditional 'are you sure?' prompts

This is a breaking change, as it affects behaviour that people might be
relying on. An entry has been added to NEWS.

As proposed on the mailing list, this change removes the unconditional
prompts on:

    o: bst workspace reset
    o: bst workspace close --remove-dir

If interactive, these commands would always interrupt you with a prompt
like this:

    This will remove all your changes, are you sure?

This seems like it may just save someone's work some time. It may also
condition folks to hit 'y' quickly without thinking.

This change also makes the non-interactive behaviour consistent with the
interactive behaviour in the default case. There is also the case of the
prompt configured by 'really-workspace-close-project-inaccessible',
which may be tackled in later work.

This change also removes the new config options to suppress those
prompts, and their associated news entry.

The relevant bit of the mailing list conversation is here:
https://mail.gnome.org/archives/buildstream-list/2018-December/msg00106.html

The issue to make interactive and non-interactive behaviour consistent
is here:
https://gitlab.com/BuildStream/buildstream/issues/744

Changes:

NEWS

@@ -50,6 +50,11 @@ buildstream 1.3.1
      an error message and a hint instead, to avoid bothering folks that just
      made a mistake.
 +  o BREAKING CHANGE: The unconditional 'Are you sure?' prompts have been
 +    removed. These would always ask you if you were sure when running
 +    'bst workspace close --remove-dir' or 'bst workspace reset'. They got in
 +    the way too often.
++
    o Failed builds are included in the cache as well.
      `bst checkout` will provide anything in `%{install-root}`.
      A build including cached fails will cause any dependant elements
@@ -87,12 +92,6 @@ buildstream 1.3.1
      instead of just a specially-formatted build-root with a `root` and `scratch`
      subdirectory.
 -  o The buildstream.conf file learned new
 -    'prompt.really-workspace-close-remove-dir' and
 -    'prompt.really-workspace-reset-hard' options. These allow users to suppress
 -    certain confirmation prompts, e.g. double-checking that the user meant to
 -    run the command as typed.
+-
    o Due to the element `build tree` being cached in the respective artifact their
      size in some cases has significantly increased. In *most* cases the build trees
      are not utilised when building targets, as such by default bst 'pull' & 'build'

buildstream/_cas/casserver.py

@@ -324,7 +324,7 @@ class _ContentAddressableStorageServicer(remote_execution_pb2_grpc.ContentAddres
              blob_response.digest.size_bytes = digest.size_bytes
              if len(blob_request.data) != digest.size_bytes:
 -                blob_response.status.code = grpc.StatusCode.FAILED_PRECONDITION
 +                blob_response.status.code = code_pb2.FAILED_PRECONDITION
                  continue
              try:
@@ -335,10 +335,10 @@ class _ContentAddressableStorageServicer(remote_execution_pb2_grpc.ContentAddres
                      out.flush()
                      server_digest = self.cas.add_object(path=out.name)
                      if server_digest.hash != digest.hash:
 -                        blob_response.status.code = grpc.StatusCode.FAILED_PRECONDITION
 +                        blob_response.status.code = code_pb2.FAILED_PRECONDITION
              except ArtifactTooLargeException:
 -                blob_response.status.code = grpc.StatusCode.RESOURCE_EXHAUSTED
 +                blob_response.status.code = code_pb2.RESOURCE_EXHAUSTED
          return response

buildstream/_context.py

@@ -121,18 +121,10 @@ class Context():
          # Whether or not to attempt to pull build trees globally
          self.pull_buildtrees = None
 -        # Boolean, whether we double-check with the user that they meant to
 -        # remove a workspace directory.
 -        self.prompt_workspace_close_remove_dir = None
+-
          # Boolean, whether we double-check with the user that they meant to
          # close the workspace when they're using it to access the project.
          self.prompt_workspace_close_project_inaccessible = None
 -        # Boolean, whether we double-check with the user that they meant to do
 -        # a hard reset of a workspace, potentially losing changes.
 -        self.prompt_workspace_reset_hard = None
+-
          # Whether elements must be rebuilt when their dependencies have changed
          self._strict_build_plan = None
@@ -260,16 +252,10 @@ class Context():
          prompt = _yaml.node_get(
              defaults, Mapping, 'prompt')
          _yaml.node_validate(prompt, [
 -            'really-workspace-close-remove-dir',
              'really-workspace-close-project-inaccessible',
 -            'really-workspace-reset-hard',
          ])
 -        self.prompt_workspace_close_remove_dir = _node_get_option_str(
 -            prompt, 'really-workspace-close-remove-dir', ['ask', 'yes']) == 'ask'
          self.prompt_workspace_close_project_inaccessible = _node_get_option_str(
              prompt, 'really-workspace-close-project-inaccessible', ['ask', 'yes']) == 'ask'
 -        self.prompt_workspace_reset_hard = _node_get_option_str(
 -            prompt, 'really-workspace-reset-hard', ['ask', 'yes']) == 'ask'
          # Load per-projects overrides
          self._project_overrides = _yaml.node_get(defaults, Mapping, 'projects', default_value={})

buildstream/_frontend/cli.py

@@ -841,11 +841,6 @@ def workspace_close(app, remove_dir, all_, elements):
          if nonexisting:
              raise AppError("Workspace does not exist", detail="\n".join(nonexisting))
 -        if app.interactive and remove_dir and app.context.prompt_workspace_close_remove_dir:
 -            if not click.confirm('This will remove all your changes, are you sure?'):
 -                click.echo('Aborting', err=True)
 -                sys.exit(-1)
+-
          for element_name in elements:
              app.stream.workspace_close(element_name, remove_dir=remove_dir)
@@ -879,11 +874,6 @@ def workspace_reset(app, soft, track_, all_, elements):
          if all_ and not app.stream.workspace_exists():
              raise AppError("No open workspaces to reset")
 -        if app.interactive and not soft and app.context.prompt_workspace_reset_hard:
 -            if not click.confirm('This will remove all your changes, are you sure?'):
 -                click.echo('Aborting', err=True)
 -                sys.exit(-1)
+-
          if all_:
              elements = tuple(element_name for element_name, _ in app.context.get_workspaces().list())

buildstream/_scheduler/scheduler.py

@@ -314,10 +314,10 @@ class Scheduler():
      #    job (Job): The job to spawn
+     #
      def _spawn_job(self, job):
 -        job.spawn()
          self._active_jobs.append(job)
          if self._job_start_callback:
              self._job_start_callback(job)
 +        job.spawn()
      # Callback for the cache size job
      def _cache_size_job_complete(self, status, cache_size):

buildstream/data/userconfig.yaml

@@ -112,14 +112,6 @@ logging:
+ #
  prompt:
 -  # Whether to really proceed with 'bst workspace close --remove-dir' removing
 -  # a workspace directory, potentially losing changes.
 -  #
 -  #  ask - Ask the user if they are sure.
 -  #  yes - Always remove, without asking.
 -  #
 -  really-workspace-close-remove-dir: ask
+-
    # Whether to really proceed with 'bst workspace close' when doing so would
    # stop them from running bst commands in this workspace.
+   #
@@ -127,11 +119,3 @@ prompt:
    #  yes - Always close, without asking.
+   #
    really-workspace-close-project-inaccessible: ask
+-
 -  # Whether to really proceed with 'bst workspace reset' doing a hard reset of
 -  # a workspace, potentially losing changes.
 -  #
 -  #  ask - Ask the user if they are sure.
 -  #  yes - Always hard reset, without asking.
 -  #
 -  really-workspace-reset-hard: ask

conftest.py

@@ -54,6 +54,7 @@ class IntegrationCache():
      def __init__(self, cache):
          cache = os.path.abspath(cache)
 +        os.makedirs(cache, exist_ok=True)
          # Use the same sources every time
          self.sources = os.path.join(cache, 'sources')

requirements/requirements.in

 -Click
 +Click >= 7.0
  grpcio >= 1.10
  Jinja2 >= 2.10
  pluginbase
 -protobuf >= 3.5
 +protobuf >= 3.6
  psutil
  # According to ruamel.yaml's PyPI page, we are suppose to use
  # "<=0.15" in production until 0.15 becomes API stable.

[Notes] [Git][BuildStream/buildstream][aevri/are_you_sure] 10 commits: requirements/requirements.in: require protobuf >= 3.6

Angelos Evripiotis pushed to branch aevri/are_you_sure at BuildStream / buildstream

Commits:

8 changed files:

Changes: