Fix pkg_tar directory entries

[jameshilliard]

Directories appear to work for zip files but not tar files.

This should ensure we don't try to process directories as normal files.

Fixes issues like:

rules_pkg/pkg/private/tar/tar_writer.py", line 242, in add_file
    with open(file_content, 'rb') as f:
IsADirectoryError: [Errno 21] Is a directory: 'bazel-out/darwin-fastbuild/bin/py/selenium/webdriver/common/devtools/v106'

[AutomatedTester]

This PR works well, I've got it in the Selenium tree for now.

[nacl]

Could you provide test cases for this?

If this is supporting what I think it is (passing in directory labels that are not TreeArtifacts), I am hesitant to support this any further, as is not a best practice. @aiuto may have a differing opinion.

Please correct me if I'm wrong, but the relevant code in the Selenium tree is a genrule that looks to be emitting a directory, which is unfortunately unsound (the documentation's words, not mine) and IIRC Bazel emits a warning saying something to this point. This IMO should instead be a first-class rule that emits a TreeArtifact (via ctx.actions.declare_directory, if you didn't already know).

[jameshilliard]

Could you provide test cases for this?

Hmm, is there a similar case that I should base one off of?

If this is supporting what I think it is (passing in directory labels that are not TreeArtifacts), I am hesitant to support this any further, as is not a best practice. @aiuto may have a differing opinion.

Well...it's apparently supported for zip files already so the inconsistency with tar files is problematic.

Please correct me if I'm wrong, but the relevant code in the Selenium tree is a genrule that looks to be emitting a directory, which is unfortunately unsound (the documentation's words, not mine) and IIRC Bazel emits a warning saying something to this point. This IMO should instead be a first-class rule that emits a TreeArtifact (via ctx.actions.declare_directory, if you didn't already know).

Yeah, I think that probably also needs to be changed, although I'm not very familiar with bazel in general so making tar work like zip files seemed easier for a short term fix.

[aiuto]

I worry about this too. It looks like it will pull in trees that might not have been staged, so it may do what you want for local builds but not remote builds. If zip does that, it is probably an accidental side effect. Both support tree artifacts as inputs.

As far as tests, there already are tests for tree artifacts in tests/tar/BUILD.

[jameshilliard]

As far as tests, there already are tests for tree artifacts in tests/tar/BUILD.

Test which reproduces the error is added.

[aiuto]

Maybe, but that case should be working already.