Split code in modules #92

Merged
merged 5 commits into from Dec 15, 2017

Conversation

Projects
None yet
3 participants
Contributor

albertodonato commented Dec 12, 2017

Split shell code in different modules.
It also renames functions and vars for FIPS, ESM and livepatch to start with the prefix for each service. This is intended to help a future refactoring, since some functions are very similar across different services.

Note that the only actual code change in the first commit is about load_modules, everything else is just moving the code to *.sh files

@albertodonato albertodonato requested a review from panlinux Dec 12, 2017

debian/install
@@ -1,3 +1,4 @@
ubuntu-advantage usr/bin/
keyrings/*.gpg usr/share/keyrings/
+modules/* usr/lib/ubuntu-advantage/modules
@panlinux

panlinux Dec 14, 2017

Contributor

Do you know what's the difference between /usr/share/ubuntu-advantage and /usr/lib/ubuntu-advantage? At first I thought /usr/lib was reserved for arch-dependent files, and /usr/share/ for the rest, so I would advocate for the ua modules to be under /usr/share. A counter example may be python, which is in /usr/lib, but it has .pyc files which I think are arch-dependent.

@albertodonato

albertodonato Dec 14, 2017

Contributor

I don't really know the policy there, I always thought lib is for "library" files, so code that can be considered modules (hence my choice here), whereas share would be for other type of data files.

If we are to believe https://en.wikipedia.org/wiki/Filesystem_Hierarchy_Standard, I would think modules could fit the definition of "Libraries for the binaries in /usr/bin and /usr/sbin.", so they could fit in /usr/lib.

Anyway, I'm fine on changing it to /usr/share if it's better suited.

@panlinux

panlinux Dec 14, 2017

Contributor

https://www.debian.org/doc/debian-policy/#file-system-hierarchy, first item of that list, makes me think /usr/share is more appropriate:

The FHS requirement that architecture-independent application-specific static files be located in /usr/share is relaxed to a suggestion. In particular, a subdirectory of /usr/lib may be used by a package (or a collection of packages) to hold a mixture of architecture-independent and architecture-dependent files. However, when a directory is entirely composed of architecture-independent files, it should be located in /usr/share.

@albertodonato

albertodonato Dec 14, 2017

Contributor

Ok, that seems more authoritative, done :)

move modules to /usr/share
Signed-off-by: Alberto Donato <alberto.donato@canonical.com>
debian/install
@@ -1,3 +1,4 @@
ubuntu-advantage usr/bin/
keyrings/*.gpg usr/share/keyrings/
+modules/* usr/share/ubuntu-advantage/modules
@panlinux

panlinux Dec 14, 2017

Contributor

Hm, I think the directory should be named after the package, i.e., ubuntu-advantage-tools, not just ubuntu-advantage.

@albertodonato

albertodonato Dec 14, 2017

Contributor

I kinda disagree, the directory contains modules for the script, there is not necessarily a 1:1 mapping between debian packages and directories under /usr/share (see zfsutils-linux: /usr/share/zfs for instance).
There could be other scripts in the same package that could have each their own subdirectory under /usr/share

@panlinux

panlinux Dec 14, 2017

Contributor

I pinged in #ubuntu-devel, let's see if there is a reply by tomorrow

@basak

basak Dec 14, 2017

there is not necessarily a 1:1 mapping between debian packages and directories under /usr/share

Agreed.

There could be other scripts in the same package that could have each their own subdirectory under /usr/share

I don't think we should go down that route in this package. That just pollutes /usr/share for the sake of it. More commonly, when the packager is in control (as is the case here with a native package), we try to use the hierarchy more to avoid potential future conflicts which can be messy to fix later. It'd be cleaner for us to arrange everything this entire project needs to do into a single directory under /usr/share, using subdirectories of that directory if needed.

I don't think it'd be a problem to use /usr/share/ubuntu-advantage for all of everything to do with "Ubuntu Advantage", including this package but also any other related package, because I don't see a potential conflict with any other package and I see it as a reasonable and responsible use of the global namespace. But by that logic multiple directories under /usr/share/ would then be inappropriate, and I'd be unhappy if you used this as a justification now and then somebody added a /usr/share/ubuntu-advantage-something-else also later.

Really this is an Archive Admin question though. This is just my opinion and shouldn't be treated as any kind of approval, disapproval or decision.

@albertodonato

albertodonato Dec 14, 2017

Contributor

@basak thanks for the extensive argument. I'd then suggest to go with @panlinux's initial suggestion and use /usr/share/ubuntu-advantage-tools. If we happen to add more scripts with their own modules, we can move these and end up with something /usr/share/ubuntu-advantage-tools/<script>/modules for each script. I don't think it's doing it now.

+1, thanks for the reorg

@panlinux panlinux merged commit 1713097 into CanonicalLtd:master Dec 15, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@albertodonato albertodonato deleted the albertodonato:split-modules branch Dec 15, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment