what's the benefit over having everything under /tmp/mount/dlc-blah/*?

Also what if the user explicitly wants to store their bin.exe under /tmp/mount/dlc-blah instead of /tmp/mount/dlc-blah/content?

> what's the benefit over having everything under /tmp/mount/dlc-blah/*?

The benefits are explained in the bug :). This allows us to have metadata files without having file collisions with the DLC content.

> Also what if the user explicitly wants to store their bin.exe under /tmp/mount/dlc-blah instead of /tmp/mount/dlc-blah/content?

Why do they want to do that? That path is not even visible to them? In other words they don't need to know about the /tmp/mount/dlc-blah. They would only know about /tmp/mount/dlc-blah/contents/ Anything before that path is not their business :) The mount point is defined by the dlc service, not them :)

what is manifest.json used for?

LICENSE file doesn't seem itself to justify a dedicated change.

This might help with the permissions issue too. It would be annoying to specify file permissions separately for every file we want to package. This would let us set the permissions for contents and everything else independently.

If we have data files in contents, we still have to iterate over all files making sure we only set +x for files that has +x before.

> what is manifest.json used for?
I remember we talked about this in the past that it would be good to keep a version of the manifest itself in the dlc for debugging purposes. Basically to see if correct dlc is donwloaded, etc.

> LICENSE file doesn't seem itself to justify a dedicated change.
I don't think this is an invasive change. Only one change in build_dlc script and one when we path the mount point to the dlc in the Install() dbus.

Sounds OK in general but this design needs to be flushed out more. e.g. how LICENSE is generated? how manifest.json is generated? How LICENSE and manifest.json ends up on disk (exact process of serving, downloading and installing)? What changes are required for this to go?

Mind writing a design proposal on this?

i agree that giving the DLC its own dedicated namespace (a content/ subdir) is a good idea.  it doesn't cost us anything, and it avoids possible future pain if (for whatever reason) we want to add more files to the DLC format.  otherwise we'd have to start doing the reverse where we create a .dlc/ subdir or something to try and avoid conflicts.

based on how the DLC is created today, i'd expect LICENSE to be part of that process (i.e. the dlc generation python script would use the existing chromite licensing code to bundle up & output the LICENSE file).

sorry, fixed the description a bit as it had some typos (dlc-path vs dlc-blah)
Anyway i think this bug can serve as its own design doc since the changes are very minor :)

it's not blocking feature (at least not blocking dlcservice). assigning to who reported. i'm more than willing to comment on your design doc.

well, its blocking something and apparently we can't ship a DLC without its LICENSE, right? This bug is not about creating the LICENSE files itself, but a way to allow these kind of things happen.

I think the proposal is in the bug description. I'll repeat it here in case it is not obvious:

1) Move all DLC files (requested by the specific dlc developers) to a separate directory in the image (e.g. contents). This can be done by replacing temp_dir with temp_dir + '/contents' in here:
https://chromium.git.corp.google.com/chromiumos/chromite/+/d2a67111a8188b180a03ebfb1b231ebefd7183df/scripts/build_dlc.py#115
and fix other minor places

2) Return the contents directory instead of the mount point to the users. This can be achieved by
 adding this statement:
mount_point_out->append("/contents")
to this line:
https://chromium.git.corp.google.com/chromiumos/platform2/+/41abfab3354d550e8fe2da94a5a820d67f4196d0/dlcservice/dlc_service_dbus_adaptor.cc#205
and do other minor fixes

I don't think we need a separate design doc for every minor changes. And Chrome OS is an open source project and we tend to keep it that way. I don't see anything special about DLCs in general that require to be secret. So feel free to add your comments here or if you think it needs internal discussion, feel free to talk offline or mark this bug as visible to Googlers only. Meanwhile let's keep the discussion here ;)

Sounds great! I'm perfectly fine with iterating designs in an open forum like here or feel free to submit CLs directly and I'm happy to take a look (may take a bit longer to review, back and forth since the end-to-end execution is not clear to reviewer).

Mind owning this end-to-end?

At least in the case of epson printer drivers, adding a contents directory will mean that a driver bundled as a DLC can't be a drop in replacement for a driver bundled as a component (since the path will have the extra "contents" component). Not an issue really, just requires a small amount of additional work on the part of whomever is responsible for migrating these things to DLC.

re #16
I don't think the problem you're describing exist! components and dlcs are built differently. From the dlc or component's perspective they don't see the content directory perse. The driver itself doesn't have a contents folder, The driver is inside the contents folder. 

escpr needs a patch (along with cups) for whatever the new mount path is.

It's not a problem, just a note. With the current DLC layout, escpr happens to find the drivers with no extra effort in /run/imageloader/epson-inkjet-printer-escpr/<version>/, which is where the DLC ends up. Assuming other components have a similar layout, they might be able to find whatever files they need in a DLC without additional changes.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/2c059d608f56b77df34e2f370ea38fc23ba05aca

commit 2c059d608f56b77df34e2f370ea38fc23ba05aca
Author: Amin Hassani <ahassani@chromium.org>
Date: Fri Jan 18 04:20:49 2019

dlcservice: Change the return path in Install from mount point to root

Currently the Install function returns the mount point of the DLC. However, it
is better to have a separate directory inside the mountpoint that contains all
the DLC files and return that one instead. This way we can drop other necessary
files in the DLC that should not be accessed by the users of the DLC.

BUG=chromium:911228
TEST=unittests

Change-Id: I921457b8f13aec7e0310c68a7e9244f3c6da2cb8
Reviewed-on: https://chromium-review.googlesource.com/1407668
Commit-Ready: Amin Hassani <ahassani@chromium.org>
Tested-by: Amin Hassani <ahassani@chromium.org>
Reviewed-by: Nicolas Norvez <norvez@chromium.org>
Reviewed-by: Xiaochu Liu <xiaochu@chromium.org>

[modify] https://crrev.com/2c059d608f56b77df34e2f370ea38fc23ba05aca/dlcservice/utils_test.cc
[modify] https://crrev.com/2c059d608f56b77df34e2f370ea38fc23ba05aca/dlcservice/dlc_service_dbus_adaptor.cc
[modify] https://crrev.com/2c059d608f56b77df34e2f370ea38fc23ba05aca/dlcservice/dbus_adaptors/org.chromium.DlcServiceInterface.xml
[modify] https://crrev.com/2c059d608f56b77df34e2f370ea38fc23ba05aca/dlcservice/dlc_service_dbus_adaptor.h
[modify] https://crrev.com/2c059d608f56b77df34e2f370ea38fc23ba05aca/dlcservice/utils.cc
[modify] https://crrev.com/2c059d608f56b77df34e2f370ea38fc23ba05aca/dlcservice/utils.h
[modify] https://crrev.com/2c059d608f56b77df34e2f370ea38fc23ba05aca/dlcservice/dlc_service_dbus_adaptor_test.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/chromite/+/22a25eb91c9c513f9c714a2312a6dea7cb65ed1d

commit 22a25eb91c9c513f9c714a2312a6dea7cb65ed1d
Author: Amin Hassani <ahassani@chromium.org>
Date: Fri Jan 18 04:20:29 2019

build_dlc: Fix building DLC with squashfs type

We change the ownership of the tempdir that is used by squashfs to root but the
cleanup code does not handle the removal properly and it fails with permission
problems. This code adds the directory 'squashfs-root' inside the tempdir and
copies everything there instead. That way we can manually cleanup
'squashfs-root' directory before the tempdir is cleaned up.

Also set the squashfs as the default file system as for the majority of the DLCs
it would be a better option.

BUG=chromium:911228
TEST=created a squashfs dlc and mounted it, it worked fine.
TEST=unittest

Change-Id: I323bff1c36d25a60170a9828e4dee33334ed29b6
Reviewed-on: https://chromium-review.googlesource.com/1407462
Commit-Ready: Amin Hassani <ahassani@chromium.org>
Tested-by: Amin Hassani <ahassani@chromium.org>
Reviewed-by: Nicolas Norvez <norvez@chromium.org>

[modify] https://crrev.com/22a25eb91c9c513f9c714a2312a6dea7cb65ed1d/scripts/build_dlc.py

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/chromite/+/cc7ffceb868eaec59f601a56961eb1e31a525b4d

commit cc7ffceb868eaec59f601a56961eb1e31a525b4d
Author: Amin Hassani <ahassani@chromium.org>
Date: Fri Jan 18 04:20:29 2019

build_dlc: Change the root directory of dlc contents

The root directory of the DLC should be 'root' directory inside the mounted
point of the DLC module. This patch basically changes that root directory.

BUG=chromium:911228
TEST=unittest
TEST=built an image and mounted it the directories were correct.

Change-Id: I6414ee0424668c5e7dddf97f2819bff0efbe3a81
Reviewed-on: https://chromium-review.googlesource.com/1407810
Commit-Ready: Amin Hassani <ahassani@chromium.org>
Tested-by: Amin Hassani <ahassani@chromium.org>
Reviewed-by: Nicolas Norvez <norvez@chromium.org>
Reviewed-by: Xiaochu Liu <xiaochu@chromium.org>

[modify] https://crrev.com/cc7ffceb868eaec59f601a56961eb1e31a525b4d/scripts/build_dlc.py