New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 769575 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Feature



Sign in to add a comment

cros_config support for filenames

Project Member Reported by sjg@chromium.org, Sep 28 2017

Issue description

We have two separate places which deal with filenames in unibuild:

1. In the ebuild we use cros_config_host to read the filename from a property, e.g.:

		thermal {
			dptf-dv = "astronaut/dptf.dv";
		};

and use it thus:

	DPTF_FILE=$(cros_config /thermal dptf-dv)
	if [ -n "${DPTF_FILE}" ]; then
		DPTF_OPTS="-a /etc/dptf/${DPTF_FILE}"


2. In the run script, we call the subsystem-specific eclass which uses cros_config to read the same filename and pass that to the daemon:

	dptf="$(get_model_conf_value "${model}" "/thermal" "dptf-dv")"
	if [[ -n "${dptf}" ]]; then
		insinto "$(dirname /etc/dptf/${dptf})"
		doins "${FILESDIR}/${dptf}"
	fi


Both of these use /etc/dptf as the directory where the files are stored. This means that "/etc/dptf" has to appear in both the installer ebuild (e.g. chromeos-bsp-xxx) and the daemon ebuild (e.g. sys-power/dpft).

It would be better if cros_config_host and cros_config could know about this directory and output it as part of the filename.

 

Comment 1 by sjg@chromium.org, Sep 28 2017

Here's my proposal:

1. cros_config learns about filename properties and the root directory for each. So we can do:

	DPTF_FILE=$(cros_config /thermal dptf-dv)
	if [ -n "${DPTF_FILE}" ]; then
		DPTF_OPTS="-a ${DPTF_FILE}"
	fi

2. Similarly with cros_config_host, something like:

	dptf="$(cros_config_host "${dtb}" -m "${model}" "/thermal" "dptf-dv")"
	if [[ -n "${dptf}" ]]; then
		insinto "$(cros_config_host "${dtb}" --absdir -m "${model}" "/thermal" "dptf-dv")"
		doins "${FILESDIR}/${dptf}"
	fi

the --absdir argument will tell cros_config_host to get the absolute directory name referenced by the property, after adding the '/etc/dptf' prefix.

The implementation would use a table within libcros_config of properties and their associated directory paths.

Comment 2 by sjg@chromium.org, Sep 28 2017

Owner: sjg@chromium.org
proposal seems fine

Comment 4 by sjg@chromium.org, Sep 29 2017

Status: Started (was: Untriaged)

Comment 5 by sjg@chromium.org, Oct 3 2017

Cc: jclinton@chromium.org
> The implementation would use a table within libcros_config of properties and their associated directory paths.

I see some major issue with this approach that we won't be able to overcome:

* Access by node+property is no longer a consistent API: as the caller, one now has to have some special knowledge about which node+properties have special --absdir flags built into the the library. (Though, this could be worked-around by killing node/property access as an API.)

* The property-specific logic has to be duplicated across the C++ and Python implementations

* Adding new property-specific logic has to be added to two places before a property that relies on this can be deployed:
  * The README.md
  * The C/Python impls
Then, we have to wait for a SDK release for cros_config_host to gain the property-specific implementation before we can deploy the .dtsi file use (a few weeks). 

* Any package that goes through a transition of where it stores its config file prefix now has to depend on a forked, co-installable cros_config_host with support for both the old and the new way before an ebuild with the new path can be uploaded. I.e. moving from "/etc/dptf" to "/etc/dptf2" is no longer encoded solely in the .ebuild.

An alternative and much simpler solution to the desire to deduplicate /etc/dptf is to simply put this semi-constant in the .eclass shared by chromeos-bsp-xxx and the daemon ebuild sys-power/dpft and make chromeos-bsp-xxx inherit from cros-dpft.eclass.

Comment 7 by sjg@chromium.org, Oct 3 2017

1. We do need both the relative and absolute path and I think it makes sense for the caller to ask cros_config_host for which one it wants.

2. That only happened as at yesterday due to crbuf.com/771187. I did mention this as a cost of moving to Python, but I think we can make it work by sharing the list of paths (having it in one place). There will only be one per subsystem (so, say 10 paths).

3. I don't think we have to wait for cros_config_host to appear in an sdk tarball - doesn't it just rebuild when we change it? I heard this come up before, but I would like to understand it. The README.md, C/Python impls are all in the same dir, so at least they can be in the same CL.

4. Are you sure? If I change the path then it will install in the new path and at run-time will be picked up from the new path. What am I missing?

5. I did want to put it in the eclass but could not figure it out, since the eclass is not available at run-time. Still, as I mentioned above, we can share this info.

> 3. I don't think we have to wait for cros_config_host to appear in an sdk tarball - doesn't it just rebuild when we change it? I heard this come up before, but I would like to understand it. The README.md, C/Python impls are all in the same dir, so at least they can be in the same CL.

The ebuild environment is running inside of the cros_sdk chroot with tools like cros_config_host used during src_configure/src_install coming from the host chroot platform. And individual person Alice can, of course, update the property-specific logic for cros_config_host and then "emerge chromeos-config-host" (without the board suffix) to update their own chroot. But, when second person Bob syncs to ToT with this new assumption, he will not have the updated chromeos-config-host in his chroot and will not be able to build ToT until the cros_sdk tarball uprev's.

One alternative would be to not have cros_config_host in the SDK and instead have it be in the package DEPEND (not RDEPEND) of every package that needs it in src_configure/src_install. However, we would never be able to use cros_config_host from any of the phases that don't have access to the build root.

> 4. Are you sure? If I change the path then it will install in the new path and at run-time will be picked up from the new path. What am I missing?

The coupling between the SDK release cycle on second person Bob's machine (and the builders) and the ebuild uprev. They aren't in lockstep.

> 5. I did want to put it in the eclass but could not figure it out, since the eclass is not available at run-time. Still, as I mentioned above, we can share this info.

We could add repeatable, optional --template K=V flags to the dtc compiler so that the properties on the DUT are fully concrete.

Comment 9 by sjg@chromium.org, Oct 3 2017

Re 3, 4, doesn't build_packages do this? It seems to for me.
build_packages doesn't currently modify the SDK.

We could explore having it do that but it's somewhat complex. For example, what happens when cros_config_host depends on a newer version of a Python library in one of its dependencies that's also in the SDK? We'd end up with silos of the SDK which are upgraded outside of the normal SDK releases process. Thought, it might be worth thinking through the value that having an SDK release process is providing. Perhaps the answer is: not much.

I'm also not sure if there are any read-only restrictions on the builders' SDK roots.

Hrm, actually, I stand corrected: we are currently updating the SDK after sync from ToT if and only if the package is *not* considered part of the toolchain *or* the package was cros_workon'd:

http://cs/chromeos_public/src/scripts/update_chroot?l=99&rcl=a595deb61b1b1805837f8b9dbbc832b8cda7d8c1

this is called from setup_board here:

http://cs/chromeos_public/src/scripts/setup_board?l=108&rcl=a595deb61b1b1805837f8b9dbbc832b8cda7d8c1

The way I'm reading this: so long as the package is in the dependencies of target-sdk, ToT will update. However, if someone adds cros_config_host to the toolchain deps, it will stop updating.

Ultimately, the SDK tarball build is providing an optimization to avoid building compilers or to accelerate the creation of the chroot on new installs.

So, that resolves 3 and 4. Sorry; I thought I understood the SDK better than I do.

Cc: pbe...@chromium.org

Comment 13 by sjg@chromium.org, Oct 17 2017

Labels: -Pri-1 Pri-2

Comment 14 by sjg@chromium.org, Oct 17 2017

Labels: -Pri-2 Pri-1

Comment 15 by sjg@chromium.org, Oct 17 2017

Summary: cros_config support for filenames (was: cros_config support for fienames)
I thought that the config files referenced in the models were going to be completely decoupled from where they'll be consumed in the end.
E.g. there is no need to actually copy them under /etc/dptf if the call to cros_config returns the full path to the file/directory. Of course http://cs/chromeos_public/src/third_party/chromiumos-overlay/sys-power/dptf/files/dptf.conf will have to be slightly adjusted (and other components similarly).

My proposal is then to stick all the config files into /usr/share/cromeos-config/files and adjust the various init scripts accordingly to just rely on the full path returned.

Comment 17 by sjg@google.com, Oct 18 2017

Are you suggesting that we put (e.g.) the files in /etc/cras into that directory instead, with a symlink to from /etc/cras? I'm not sure I follow.

No, that's not what I'm suggesting. Taking the concrete example from dptf, I would see its script changed as below. ${DPTF_FILE} can be "/usr/share/chromeos-config/files/Thermal/my_wonderful_model_dptf.dv". The script shouldn't need to have some magic portion of path to file as prefix, as otherwise it leads to your initial problem statement. I thought that's what Charles had in mind too, but maybe I misunderstood him. 

script
	DPTF_OPTS=""
	DPTF_FILE="$(cros_config /thermal dptf-dv)" || true
	if [ -n "${DPTF_FILE}" ]; then
		dptf="${DPTF_FILE}" <-- Note the removal of /etc/dptf/ in front of ${DPTF_FILE}!

		if [ ! -f "${dptf}" ]; then
			logger -i -t "${UPSTART_JOB}" "Failed to find ${dptf}"
			exit 1
		fi

		DPTF_OPTS="-a ${dptf}"
	fi
	exec esif_ufd -n ${DPTF_OPTS}
end script

Comment 19 by sjg@chromium.org, Oct 18 2017

Yes that is my intent with this bug. It is how touchscreen works - i.e. cros_config_host has the full path. However the support in cros_config has not been done yet. I'll try to get a draft impl tomorrow.
Project Member

Comment 20 by bugdroid1@chromium.org, Oct 20 2017

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

commit a510643adb7f3d1b17c6df56dea3bf7069549022
Author: Simon Glass <sjg@chromium.org>
Date: Fri Oct 20 08:51:06 2017

chromeos-config: Rename AudioFile to BaseFile

We plan to use this object for things other than audio. For example it can
be used for thermal files. Rename it to 'BaseFile' to reflect that.

BUG= chromium:769575 
TEST=PYTHONPATH=~/cosarm python cros_config_host_py/libcros_config_host_unittest.py

Change-Id: I070ba81cd661eb77f08fab222b3e0d6eb235b3d2
Signed-off-by: Simon Glass <sjg@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/729134
Reviewed-by: Jason Clinton <jclinton@chromium.org>

[modify] https://crrev.com/a510643adb7f3d1b17c6df56dea3bf7069549022/chromeos-config/cros_config_host_py/libcros_config_host_unittest.py
[modify] https://crrev.com/a510643adb7f3d1b17c6df56dea3bf7069549022/chromeos-config/cros_config_host_py/cros_config_host.py
[modify] https://crrev.com/a510643adb7f3d1b17c6df56dea3bf7069549022/chromeos-config/cros_config_host_py/libcros_config_host.py

Project Member

Comment 21 by bugdroid1@chromium.org, Oct 20 2017

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

commit 80a526db7f3c25f3492bcb0aa0e9ba29452949b4
Author: Simon Glass <sjg@chromium.org>
Date: Fri Oct 20 08:51:07 2017

chromeos-config: Pass schema to CrosConfigValidator constructor

Rather than passing the schema to the Start() method, pass it to the
constructor. We are not planning to pass different schemas, and this
allows us to return a CrosConfigValidator with the schema set up and
ready to use.

Add a GetValidator() function for this. This will be called by
libcros_config_host to access schema information.

BUG= chromium:769575 
TEST=PYTHONPATH=~/cosarm \
   python cros_config_host_py/libcros_config_host_unittest.py;
PYTHONPATH=~/cosarm \
   python validate/validate_config_unittest.py

Change-Id: I06ce2683b7fa4309d6bc9e19825fdb2384d0cf7c
Signed-off-by: Simon Glass <sjg@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/729135
Reviewed-by: Jason Clinton <jclinton@chromium.org>

[modify] https://crrev.com/80a526db7f3c25f3492bcb0aa0e9ba29452949b4/chromeos-config/validate/validate_config.py
[modify] https://crrev.com/80a526db7f3c25f3492bcb0aa0e9ba29452949b4/chromeos-config/validate/validate_config_unittest.py

Project Member

Comment 22 by bugdroid1@chromium.org, Oct 20 2017

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

commit c0d418459a42a24b692f7958f579bed6cb105093
Author: Simon Glass <sjg@chromium.org>
Date: Fri Oct 20 08:51:07 2017

chromeos-config: Access the schema from libcros_config_host

We will use the schema to access information about properties, in
particular the upcoming target_dir property. Set this up.

For running locally (within the platform2/chromeos-config directory) we
need to fiddle the path to permit access to the validator. Once everything
is in one place we will be able to drop this. See  crbug.com/774492 

BUG= chromium:769575 
TEST=PYTHONPATH=~/cosarm \
   python cros_config_host_py/libcros_config_host_unittest.py;
PYTHONPATH=~/cosarm \
   python validate/validate_config_unittest.py

Change-Id: I0739facf88bd710ad0c19c69dc8c8fb9424e2306
Signed-off-by: Simon Glass <sjg@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/729136
Reviewed-by: Jason Clinton <jclinton@chromium.org>

[modify] https://crrev.com/c0d418459a42a24b692f7958f579bed6cb105093/chromeos-config/cros_config_host_py/libcros_config_host.py

Project Member

Comment 23 by bugdroid1@chromium.org, Oct 20 2017

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

commit 5571b5e8f54884b7ea048f52748bdb155c027054
Author: Simon Glass <sjg@chromium.org>
Date: Fri Oct 20 08:51:07 2017

chromeos-config: Add a target directory to the schema

Some files need to be installed in the root. Rather than having the path
for that in two places (the ebuild and the init script, for example), we
can build this information into cros_config. Then cros_config_host and
cros_config can return the absolute path, both from the same source.

Add a new schema property for this.

BUG= chromium:769575 
TEST=PYTHONPATH=~/cosarm \
   python cros_config_host_py/libcros_config_host_unittest.py;
PYTHONPATH=~/cosarm \
   python validate/validate_config_unittest.py

Change-Id: I5fcb072109c5515e5ed64273abb81ca2a99da260
Signed-off-by: Simon Glass <sjg@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/729137
Reviewed-by: Jason Clinton <jclinton@chromium.org>

[modify] https://crrev.com/5571b5e8f54884b7ea048f52748bdb155c027054/chromeos-config/validate/validate_config.py
[modify] https://crrev.com/5571b5e8f54884b7ea048f52748bdb155c027054/chromeos-config/validate/validate_schema.py

Project Member

Comment 24 by bugdroid1@chromium.org, Oct 20 2017

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

commit e879e975bc322dcfd71678f9afcebfe3b2b65c2d
Author: Simon Glass <sjg@chromium.org>
Date: Fri Oct 20 08:51:07 2017

chromeos-config: Provide a way to access schema target_dir

Add a function to provide the target directory from the schema for a
particular schema element. This will be used by cros_config_host.

Note: This needs a test, not yet done.

BUG= chromium:769575 
   TEST=PYTHONPATH=~/cosarm \
python cros_config_host_py/libcros_config_host_unittest.py;
PYTHONPATH=~/cosarm \
   python validate/validate_config_unittest.py

Change-Id: I05a26b1630ac9c8524d99ff980b9e014cecc008f
Signed-off-by: Simon Glass <sjg@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/729138
Reviewed-by: Jason Clinton <jclinton@chromium.org>

[modify] https://crrev.com/e879e975bc322dcfd71678f9afcebfe3b2b65c2d/chromeos-config/validate/validate_config.py

Project Member

Comment 25 by bugdroid1@chromium.org, Oct 21 2017

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

commit 5afab567ec4ead5bc3a0d0899e3cd4bd0a7bf395
Author: Simon Glass <sjg@chromium.org>
Date: Sat Oct 21 06:55:09 2017

chromeos-config: Add support for thermal files

Add support for obtaining a list of thermal files for a family. This
provides the relative path for the source file (typically within
${FILESDIR}) and the absolute path for the destination file.

BUG= chromium:769575 
TEST=PYTHONPATH=~/cosarm \
   python cros_config_host_py/libcros_config_host_unittest.py;
PYTHONPATH=~/cosarm \
   python validate/validate_config_unittest.py

Change-Id: Ic06e3366a18037034d7e62dffda70fc383b3c523
Signed-off-by: Simon Glass <sjg@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/729199

[modify] https://crrev.com/5afab567ec4ead5bc3a0d0899e3cd4bd0a7bf395/chromeos-config/libcros_config/test.dts
[modify] https://crrev.com/5afab567ec4ead5bc3a0d0899e3cd4bd0a7bf395/chromeos-config/cros_config_host_py/libcros_config_host_unittest.py
[modify] https://crrev.com/5afab567ec4ead5bc3a0d0899e3cd4bd0a7bf395/chromeos-config/cros_config_host_py/cros_config_host.py
[modify] https://crrev.com/5afab567ec4ead5bc3a0d0899e3cd4bd0a7bf395/chromeos-config/cros_config_host_py/libcros_config_host.py

Project Member

Comment 26 by bugdroid1@chromium.org, Oct 23 2017

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

commit c72f155a86201d487b40c862d4523656cdf3e815
Author: Simon Glass <sjg@chromium.org>
Date: Mon Oct 23 22:22:16 2017

chromeos-config: Add a schema object for filenames

The target_path property is currently a core element property. But so far
it is only used with a small number of elements. It seems better to have
it as a new element type with its own setting.

Create a PropFile schema element for this.

BUG= chromium:769575 
TEST=FEATURES=test emerge-reef-uni --nodeps chromeos-config-tools

Change-Id: Ia3bf18e54a03ed5b230aaf9f5bb244b4a5918835
Signed-off-by: Simon Glass <sjg@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/732319
Reviewed-by: Jason Clinton <jclinton@chromium.org>

[modify] https://crrev.com/c72f155a86201d487b40c862d4523656cdf3e815/chromeos-config/validate_config.py
[modify] https://crrev.com/c72f155a86201d487b40c862d4523656cdf3e815/chromeos-config/validate_schema.py

Project Member

Comment 27 by bugdroid1@chromium.org, Oct 23 2017

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

commit c202c4e6f834789f2076515f132562ab0efafa89
Author: Simon Glass <sjg@chromium.org>
Date: Mon Oct 23 22:22:16 2017

chromeos-config: Adjust audio to use PropFile

Use this new schema object for audio properties.

Also provide a 'model' property which can be used in the audio-type node
to avoid repeating this for every model.

BUG= chromium:769575 
TEST=FEATURES=test emerge-reef-uni --nodeps chromeos-config-tools

Change-Id: I251ec01804c0bf902e5c1dbd012ee250e2f79c22
Signed-off-by: Simon Glass <sjg@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/732320
Reviewed-by: Jason Clinton <jclinton@chromium.org>

[modify] https://crrev.com/c202c4e6f834789f2076515f132562ab0efafa89/chromeos-config/libcros_config_host/libcros_config_host.py
[modify] https://crrev.com/c202c4e6f834789f2076515f132562ab0efafa89/chromeos-config/validate_config.py
[modify] https://crrev.com/c202c4e6f834789f2076515f132562ab0efafa89/chromeos-config/validate_schema.py

Project Member

Comment 28 by bugdroid1@chromium.org, Oct 24 2017

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

commit 2e77a986d50781821c53686ae1f92c6c182bd9f0
Author: Simon Glass <sjg@chromium.org>
Date: Tue Oct 24 12:42:01 2017

chromeos-config: Add a PropAny schema element

Sometimes we have properties which we don't know the name of, and we don't
care to validate. An example is the schema target directory node. This is
generated by the system so validation is not necessary.

BUG= chromium:769575 
TEST=FEATURES=test emerge-reef-uni --nodeps chromeos-config-tools

Change-Id: I36698ece89d16e0e6943f51874195ed39ab23fcc
Signed-off-by: Simon Glass <sjg@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/734440
Reviewed-by: Jason Clinton <jclinton@chromium.org>

[modify] https://crrev.com/2e77a986d50781821c53686ae1f92c6c182bd9f0/chromeos-config/validate_config.py
[modify] https://crrev.com/2e77a986d50781821c53686ae1f92c6c182bd9f0/chromeos-config/validate_schema.py

Project Member

Comment 29 by bugdroid1@chromium.org, Oct 24 2017

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

commit 408a2ba0ab6ee00214b665be45dfaa1e91414f85
Author: Simon Glass <sjg@chromium.org>
Date: Tue Oct 24 14:29:41 2017

chromeos-config: Generate the target dirs config

At present the target directory information is in the schema and thus is
only accessible to the Python (host) side. It cannot be used at run-time.
We want to add support for cros_config to add the path to the values it
returns at run-time.

Add an option to cros_config_host to write this information to a .dtsi
file so that it can be included in the master configuration. In this way,
it can be accessed by cros_config.

BUG= chromium:769575 
TEST=FEATURES=test emerge-reef-uni --nodeps chromeos-config-tools

Signed-off-by: Simon Glass <sjg@chromium.org>

Change-Id: Ie53ec6a30ab772aff081bd77f516fff98b256f0d
Reviewed-on: https://chromium-review.googlesource.com/732321
Commit-Ready: Simon Glass <sjg@chromium.org>
Tested-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Simon Glass <sjg@chromium.org>

[modify] https://crrev.com/408a2ba0ab6ee00214b665be45dfaa1e91414f85/chromeos-config/libcros_config/test.dts
[modify] https://crrev.com/408a2ba0ab6ee00214b665be45dfaa1e91414f85/chromeos-config/libcros_config_host/libcros_config_host.py
[modify] https://crrev.com/408a2ba0ab6ee00214b665be45dfaa1e91414f85/chromeos-config/cros_config_host_unittest.py
[modify] https://crrev.com/408a2ba0ab6ee00214b665be45dfaa1e91414f85/chromeos-config/validate_config_unittest.py
[modify] https://crrev.com/408a2ba0ab6ee00214b665be45dfaa1e91414f85/chromeos-config/libcros_config_host/fdt_util.py
[modify] https://crrev.com/408a2ba0ab6ee00214b665be45dfaa1e91414f85/chromeos-config/cros_config_host.py
[modify] https://crrev.com/408a2ba0ab6ee00214b665be45dfaa1e91414f85/chromeos-config/validate_config.py
[modify] https://crrev.com/408a2ba0ab6ee00214b665be45dfaa1e91414f85/chromeos-config/libcros_config_host_unittest.py

Project Member

Comment 30 by bugdroid1@chromium.org, Oct 24 2017

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

commit c48a348d5502c32020d6d759f25cb0c294879bc8
Author: Simon Glass <sjg@chromium.org>
Date: Tue Oct 24 14:29:41 2017

chromeos-config: Add a --abspath flag to cros_config

Use the target-directory map provided by the ebuild to make cros_config
aware of directory paths at run-time. This allows us to centralise the
use of paths in the config system rather than having the same path
repeated in the build system (e.g. ebuild) and the init scripts. We can
then be more sure that the correct path is used at run-time and thus that
the file will be found at run-time.

Update the Fake with this new interface.

BUG= chromium:769575 
TEST=FEATURES=test emerge-reef-uni --nodeps chromeos-config-tools

Change-Id: I2c1693e6ee488ca5a16a1b981c0115aca6325d5c
Reviewed-on: https://chromium-review.googlesource.com/732645
Commit-Ready: Simon Glass <sjg@chromium.org>
Tested-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Simon Glass <sjg@chromium.org>

[modify] https://crrev.com/c48a348d5502c32020d6d759f25cb0c294879bc8/chromeos-config/cros_config_main.cc
[modify] https://crrev.com/c48a348d5502c32020d6d759f25cb0c294879bc8/chromeos-config/libcros_config/cros_config.cc
[modify] https://crrev.com/c48a348d5502c32020d6d759f25cb0c294879bc8/chromeos-config/libcros_config/fake_cros_config.cc
[modify] https://crrev.com/c48a348d5502c32020d6d759f25cb0c294879bc8/chromeos-config/libcros_config/fake_cros_config.h
[modify] https://crrev.com/c48a348d5502c32020d6d759f25cb0c294879bc8/chromeos-config/libcros_config/cros_config_interface.h
[modify] https://crrev.com/c48a348d5502c32020d6d759f25cb0c294879bc8/chromeos-config/libcros_config/cros_config_unittest.cc
[modify] https://crrev.com/c48a348d5502c32020d6d759f25cb0c294879bc8/chromeos-config/libcros_config/fake_cros_config_unittest.cc
[modify] https://crrev.com/c48a348d5502c32020d6d759f25cb0c294879bc8/chromeos-config/cros_config_main_unittest.cc
[modify] https://crrev.com/c48a348d5502c32020d6d759f25cb0c294879bc8/chromeos-config/libcros_config/cros_config.h

Project Member

Comment 31 by bugdroid1@chromium.org, Oct 24 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/57cfab275dff0c49b110318ec881b405dc830852

commit 57cfab275dff0c49b110318ec881b405dc830852
Author: Simon Glass <sjg@chromium.org>
Date: Tue Oct 24 17:38:25 2017

chromeos-config: Generate the target directory map

This is needed by cros_config to be able to provide absolute paths at
run-time. Generate the configuratoin fragment file by calling
cros_config_host, and install it as part of the master config.

BUG= chromium:769575 
CQ-DEPEND=CL:732321
TEST=With other changes:
FEATURES=test emerge-reef-uni --nodeps chromeos-config-tools

Change-Id: I42e330186db2ff38338b55e40fd3a9fffa901ded
Signed-off-by: Simon Glass <sjg@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/732311

[modify] https://crrev.com/57cfab275dff0c49b110318ec881b405dc830852/chromeos-base/chromeos-config/chromeos-config-0.0.1.ebuild
[rename] https://crrev.com/57cfab275dff0c49b110318ec881b405dc830852/chromeos-base/chromeos-config/chromeos-config-0.0.1-r6.ebuild

Project Member

Comment 32 by bugdroid1@chromium.org, Nov 1 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/01447c99092d52a312c52593550b2efd09bda0d3

commit 01447c99092d52a312c52593550b2efd09bda0d3
Author: Simon Glass <sjg@chromium.org>
Date: Wed Nov 01 18:26:01 2017

chromeos-config-tools: Validate the binding

Run the validator on the master configuration binding to ensure it does
not show an incorrect example.

BUG= chromium:769575 
TEST=FEATURES=test sudo emerge -E --nodeps chromeos-config-tools
(after inserting an error in the touch node in the README.md example)
See that it fails to build now:

 * Validating master configuration binding
README.md:
/chromeos/models/reef/touch: 'present' value 'probex' does not match
	pattern '^yes|no|probe$'

Change-Id: I229fbc298637fa4ea1c1842883211c52fac59d65
Signed-off-by: Simon Glass <sjg@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/742469
Reviewed-by: Jason Clinton <jclinton@chromium.org>

[modify] https://crrev.com/01447c99092d52a312c52593550b2efd09bda0d3/chromeos-base/chromeos-config-tools/chromeos-config-tools-9999.ebuild

Comment 33 by sjg@chromium.org, Nov 4 2017

Labels: Unibuild

Comment 34 by sjg@chromium.org, Nov 6 2017

Status: Fixed (was: Started)

Sign in to add a comment