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

incompatible_depset_is_not_iterable: Make depset not iterable #5816

Closed
c-parsons opened this issue Aug 8, 2018 · 13 comments
Closed

incompatible_depset_is_not_iterable: Make depset not iterable #5816

c-parsons opened this issue Aug 8, 2018 · 13 comments
Assignees
Labels
incompatible-change Incompatible/breaking change P1 I'll work on this now. (Assignee required)

Comments

@c-parsons
Copy link
Contributor

This is a tracking issue for offering a migration solution for
--incompatible_depset_is_not_iterable

This flag would enforce that depset is no longer treatable as an iterable. Thus, len(depset()) would throw an error.
This is useful in it prevents users from using this gravely-inefficient construct (expansion of depsets should be avoidable at all costs, as they are incredibly expensive operations)

@c-parsons c-parsons self-assigned this Aug 8, 2018
@dslomov dslomov changed the title Make depset not iterable incompatible_depset_is_not_iterable: Make depset not iterable Oct 31, 2018
@dslomov dslomov added the incompatible-change Incompatible/breaking change label Oct 31, 2018
@dslomov
Copy link
Contributor

dslomov commented Oct 31, 2018

What is missing for migration: migration docs, length of migration window. After these are done, please add "migration-ready" label

bttk added a commit to bttk/bazel-skylib that referenced this issue Nov 13, 2018
With this change tests pass even if
--incompatible_depset_is_not_iterable is used.

Bazel issue:
bazelbuild/bazel#5816
bttk added a commit to bttk/bazel-skylib that referenced this issue Nov 13, 2018
With this change tests pass even if
--incompatible_depset_is_not_iterable is used.

Bazel issue:
bazelbuild/bazel#5816
bttk added a commit to bttk/bazel-skylib that referenced this issue Nov 13, 2018
With `--incompatible_depset_is_not_iterable` depset is no longer
iterable. Remove tests that assumed it is.

Bazel issue:
bazelbuild/bazel#5816
laurentlb pushed a commit to bazelbuild/bazel-skylib that referenced this issue Nov 14, 2018
With `--incompatible_depset_is_not_iterable` depset is no longer
iterable. Remove tests that assumed it is.

Bazel issue:
bazelbuild/bazel#5816
@laurentlb laurentlb added P1 I'll work on this now. (Assignee required) team-Starlark and removed category: extensibility > skylark labels Nov 21, 2018
@manekinekko
Copy link

Hi, how would you coerce a depset to a flat list without using calling .to_list() (not recommended) nor having to iterate?

@c-parsons
Copy link
Contributor Author

The recommendation is to avoid flattening a depset in Starlark at all costs -- depsets can be passed to action creation directly (via actions.run)

@m3rcuriel
Copy link
Contributor

I'm curious then as to what the recommended syntax would be for checking if a label exists in a depset, i.e what would formerly be if blah in your_depset

@c-parsons
Copy link
Contributor Author

From https://docs.bazel.build/versions/master/skylark/lib/depset.html :

Depsets are not implemented as hash sets and do not support fast membership tests. If you need a general set datatype, you can simulate one using a dictionary where all keys map to True.

You cannot check if a label exists in a depset without flattening it.

@laurentlb
Copy link
Contributor

laurentlb commented May 20, 2019

Status update. Flag flip still blocked by:

@davido
Copy link
Contributor

davido commented May 21, 2019

@laurentlb My impression was, that it is fixed in all our transitive dependencies and in our own build tool chain. Can you point me to a log, specific to this breakage?

@davido
Copy link
Contributor

davido commented May 21, 2019

Thanks. I see it now:

File "/var/lib/buildkite-agent/builds/bk-docker-n5kr/bazel-downstream-projects/gerrit/tools/bzl/junit.bzl", line 38, in _AsClassName
[x.path for x in fname.files]
type 'depset' is not iterable. Use the `to_list()` method to get a list. Use --incompatible_depset_is_not_iterable=false to temporarily disable this check.

I filed: [1].

bazel-io pushed a commit that referenced this issue May 21, 2019
Progress towards #5816

RELNOTES: None.
PiperOrigin-RevId: 249341642
bazel-io pushed a commit that referenced this issue Jun 3, 2019
RELNOTES[INC]: Depsets can't be iterated over unless they're converted to lists
using the .to_list() method. Use --incompatible_depset_is_not_iterable=false to
temporarily restore the previous behaviour.

#5816

PiperOrigin-RevId: 251249558
irengrig pushed a commit to irengrig/bazel that referenced this issue Jun 18, 2019
Progress towards bazelbuild#5816

RELNOTES: None.
PiperOrigin-RevId: 249341642
irengrig pushed a commit to irengrig/bazel that referenced this issue Jun 18, 2019
RELNOTES[INC]: Depsets can't be iterated over unless they're converted to lists
using the .to_list() method. Use --incompatible_depset_is_not_iterable=false to
temporarily restore the previous behaviour.

bazelbuild#5816

PiperOrigin-RevId: 251249558
pmuetschard added a commit to pmuetschard/gapid that referenced this issue Jun 21, 2019
pmuetschard added a commit to pmuetschard/gapid that referenced this issue Jun 21, 2019
pmuetschard added a commit to google/gapid that referenced this issue Jun 24, 2019
irengrig pushed a commit to irengrig/bazel that referenced this issue Jul 15, 2019
RELNOTES[INC]: Depsets can't be iterated over unless they're converted to lists
using the .to_list() method. Use --incompatible_depset_is_not_iterable=false to
temporarily restore the previous behaviour.

bazelbuild#5816

PiperOrigin-RevId: 251249558
@lberki
Copy link
Contributor

lberki commented Jul 22, 2019

This broke rules_scala (no Travis link, since I realized this after updating it for two other incompatible flags, which also broke). How was this flag flipped then?

(The story (still developing) is under bazelbuild/rules_scala#793; I hope that this was the last flag I have to fix up...)

@laurentlb
Copy link
Contributor

It was discussed by email, but for the record: we test Bazel on BuildKite on many projects.

It looks like rules_scala is not fully tested on BuildKite (some tests are disabled), which can hide some issues.

bazel-io pushed a commit that referenced this issue Nov 15, 2019
#5816

RELNOTES: None.
PiperOrigin-RevId: 280688754
laszlocsomor added a commit to laszlocsomor/rules_typescript that referenced this issue Nov 26, 2019
Remove this flag from the .bazelrc.

Bazel no longer supports this flag, see bazelbuild/bazel#5816
@Magsun
Copy link

Magsun commented Jan 6, 2020

Hello to anyone who sees this issue report, I would be so happy if someone can help.
I met this unrecognized option error while I'm trying to convert local video to yt8m format features.
The command line is listed below:
bazel build -c opt \
--define MEDIAPIPE_DISABLE_GPU=1 --define no_aws_support=true
mediapipe/examples/desktop/youtube8m:extract_yt8m_features

The whole info of this error is:
INFO: Options provided by the client:
Inherited 'common' options: --isatty=1 --terminal_columns=168
INFO: Reading rc options for 'build' from /workspace/paddlevideo/mediapipe/.bazelrc:
'build' options: --jobs 128 --define=absl=1 --cxxopt=-std=c++14 --copt=-Wno-sign-compare --copt=-Wno-unused-function --copt=-Wno-uninitialized --copt=-Wno-unused-result --copt=-Wno-comment --copt=-Wno-return-type --copt=-Wno-unused-local-typedefs --copt=-Wno-ignored-attributes --incompatible_disable_deprecated_attr_params=false --incompatible_depset_is_not_iterable=false --apple_platform_type=macos --apple_generate_dsym
ERROR: Unrecognized option: --incompatible_depset_is_not_iterable=false

My bazel version is 2.0.0 installed by binary .sh file, and sys version is Ubuntu 16.04.
FFMPEG and OPENCV are installed.
Local video to yt8m:https://github.com/google/mediapipe/tree/master/mediapipe/examples/desktop/youtube8m

@laurentlb
Copy link
Contributor

The flag incompatible_depset_is_not_iterable has been removed (it was useful only for the transition) and shouldn't be used anymore.

Can you remove it? It looks like it comes from /workspace/paddlevideo/mediapipe/.bazelrc.

If the flag was really needed (i.e. you get errors related to depset iteration), you may have to fix the code before using Bazel 2.0. This should work with Bazel 1.2 though.

jparise added a commit to pinterest/tulsi that referenced this issue Sep 9, 2021
luca-digrazia pushed a commit to luca-digrazia/DatasetCommitsDiffSearch that referenced this issue Sep 4, 2022
luca-digrazia pushed a commit to luca-digrazia/DatasetCommitsDiffSearch that referenced this issue Sep 4, 2022
    Progress towards bazelbuild/bazel#5816

    RELNOTES: None.
    PiperOrigin-RevId: 249341642
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
incompatible-change Incompatible/breaking change P1 I'll work on this now. (Assignee required)
Projects
None yet
Development

No branches or pull requests

9 participants