Provide pkg_mklinks for creation of in-package symbolic links

[nacl]

Symbolic links are often included within packages, referring to files and
directories within and without.

This commit provides a rule, pkg_mklinks, which allows for the creation of
arbitrary symbolic links within a package, and support within the experimental
pkg_rpm rule to emit them.

buildifier was also opportunistically applied to files in this change.

Tests for integrating with the experimental RPM packager are available at nacl@352899c, and can be added to this PR after #160 is merged.

[aiuto]

I think what we need before continuing with things like this is a good example of how they would be used to achieve desired effects. This current implementation looks like it would only work for RPM.

What I have seen people need though, is more complex things like

bar/BUILD:

cc_binary(name = "foo",  data=["//some/other/place:data"])

pkg_filegroup(
    name="dist",
    content = [
       pkg_file(src=":foo", dest=foo_bin/foo, attr=...)
       pkg_file(src="//some/other/place:data", dest=foo_bin/data)
       pkg_link(name="foo", dest="foo_bin/foo")
   ]
)

The above would, hypothetically, describe a unit of 3 files. Now, that should be consumable by another filegroup in another location (lets say the place where we gather foo and a few other programs), And finally the pkg_rpm should just be able to depend on that intermediate.

[nacl]

Very interesting. Unfortunately, I've been doing some high-priority internal porting work that has prevented me from looking at these reviews. Sorry for not being responsive. I hope to be able to start taking a look at these again later in July.

[aiuto]

No problem. I have been super busy myself.

[nacl]

I think what we need before continuing with things like this is a good example of how they would be used to achieve desired effects.

Yeah, I've been a bit behind in this and haven't been able to explore using the existing implementation for, say, a tarfile or zip builder.

This current implementation looks like it would only work for RPM.

The implementation of pkg_filegroup or pkg_mklinks? Both?

Regardless, how'd you come to this conclusion? It could very well end up being that way; I'm honestly not sure.

What I have seen people need though, is more complex things like

bar/BUILD:

cc_binary(name = "foo", data=["//some/other/place:data"])

pkg_filegroup(
    name="dist",
    content = [
        pkg_file(src=":foo", dest=foo_bin/foo, attr=...)
        pkg_file(src="//some/other/place:data", dest=foo_bin/data)
        pkg_link(name="foo", dest="foo_bin/foo")
    ]
)

The above would, hypothetically, describe a unit of 3 files. Now, that should be consumable by another filegroup in another location (lets say the place where we gather foo and a few other programs), And finally the pkg_rpm should just be able to depend on that intermediate.

I agree composability of pkg_filegroups is a good thing to have, but it hasn't so far been necessary for the use cases I've seen, however limited they may be.

However, I wonder how this could be implemented. Is it possible to have a macro like this:

def pkg_file(**kwargs):
    unique_name = _get_name_somehow() # from the args perhaps
    _some_rule_that_defines_the_package_mapping(name = unique_name, **kwargs)
    return ":" + unique_name

and use it in a rule? I honestly never tried. Such an implementation may indeed be easier to use, but may make debugging harder.

How would one get access to the pkg_file mapping's name? Rules like that could perhaps benefit from being given anonymous names; line numbers may be enough for users to debug issues with their implementations.

[aiuto]

Sorry for the slow reply. I've been tied up with things for the last few days and am about to head out for a short vacation. I can take a look again on the 22nd.

[aiuto]

cc/ @dannysullivan

Fortuitously, I just saw a solution to your quesiton of how to get the pkg_file mapping. We could use a provider for that. Something like

pkg_file = provider(fields={src, dest, attr, ...})
pkg_link = provider(fields={name, dest})

def pkg_filegroup(name, content):
  ...
  n_inputs = 0
  input_targets = []
  for target in content:
     if pkg_file in target:
        f_target = target[pkg_file]
           src = getattr(f_target, "src")
           attr = getattr(f_target, "attr", "0444")
           ... use src & attr as attributes to the pkg_filegroup invocation we are building
           ... OR have a _pkg_file rule which takes f_target and just echos the provider data
                     create a target of that rule here and add that target to srcs for pkg_filegroup
           target_name = 'pkg_src_%d' % n_inputs
           n_inputs += 1
           _pkg_file(name = target_name, file=f_target)
           input_targets.append(':' + target_name)
    if pkg_link in target:
        ... 

[nacl]

@aiuto I feel like this change can probably go in as-is and we can begin some broader design work. WDYT?

As for that, I've been working on a quick mockup for a more hierarchical pkg_filegroup implementation that can be used as a discussion-starter. Where do you think the discussion should be started? In the meantime, I created a draft review in #212.

[dannysullivan]

Merging as-is and moving the broader design conversation elsewhere sounds good to me - @aiuto, any thoughts?

[aiuto]

I guess I am OK with that, because it is still in experimental. Can you take care of the merge to see if that works correctly?

…

On Fri, Aug 7, 2020 at 11:14 AM Daniel Sullivan ***@***.***> wrote: Merging as-is and moving the broader design conversation elsewhere sounds good to me - @aiuto <https://github.com/aiuto>, any thoughts? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#175 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAXHHHHP4INNLEOBCQWV2ITR7QK4PANCNFSM4M56HXSA> .

[nacl]

I guess I am OK with that, because it is still in experimental. Can you take care of the merge to see if that works correctly?

Done, thanks! Looks good on my end.