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

Convert depset into a list before iterating #67

Closed

Conversation

bttk
Copy link
Contributor

@bttk bttk commented Nov 13, 2018

With this change tests pass even if --incompatible_depset_is_not_iterable is used.

Bazel issue:
bazelbuild/bazel#5816

With this change tests pass even if
--incompatible_depset_is_not_iterable is used.

Bazel issue:
bazelbuild/bazel#5816
lib/new_sets.bzl Show resolved Hide resolved
@@ -43,6 +43,34 @@ def _precondition_only_sets_or_lists(*args):
fail("Expected arguments to be depset or list, but found type %s: %r" %
(t, a))

def _to_set(a):
Copy link
Contributor

Choose a reason for hiding this comment

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

cc @c-parsons
I'd prefer to delete this file, rather than maintain it.

Copy link
Contributor Author

@bttk bttk Nov 13, 2018

Choose a reason for hiding this comment

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

I'd prefer to delete this file, rather than maintain it.

Is there a migration plan for current users? I don't see a deprecation warning in the file.

For example:

  1. add print("DEPRECATED: ... at the top of sets.bzl
  2. Require an action from existing users:
    • move sets.bzl to a deprecated subdirectory
    • rename sets = struct( to sets_deprecated = struct(

I have tried to use an alias() to add a deprecation warning, but it doesn't seem to be possible.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The first step will be done with #70

@thomasvl
Copy link
Member

Isn't this overlapping with #54 which is waiting on final signoff/changes?

@bttk
Copy link
Contributor Author

bttk commented Nov 13, 2018

Isn't this overlapping with #54 which is waiting on final signoff/changes?

Waiting for almost 3 weeks now.

@@ -32,7 +32,7 @@ def _bzl_library_impl(ctx):
# a separate program, or from `tools` of a genrule().
DefaultInfo(
files = all_files,
runfiles = ctx.runfiles(files = list(all_files)),
runfiles = ctx.runfiles(transitive_files = all_files),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have moved this change to #69

@bttk bttk closed this Nov 14, 2018
@bttk bttk deleted the incompatible_depset_is_not_iterable branch November 14, 2018 17:40
@bttk bttk mentioned this pull request Nov 16, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants