Skip to content

Commit

Permalink
Ignore empty dtoverlay coming from target state
Browse files Browse the repository at this point in the history
Before v16, the supervisor would incorrectly interpret an empty dtoverlay in
config.txt (i.e. a line with `dtoverlay=`) as a proper variable and
create it as config var if found on config.txt before provisioning.
With the latest supervisor, an empty dtoverlay on config.txt is
interpreted in the [correct way](https://www.raspberrypi.com/documentation/computers/config_txt.html#dtoverlay), but
that makes an empty dtoverlay on the target state meaningless, leading
to looping behavior as the current and target state won't match.

This PR modifies the pre-processing function to filter out empty array
variables before the comparison, to prevent this sort of looping
behavior. This allows empty dtoverlay to be ignored.

Change-type: patch
  • Loading branch information
pipex committed Apr 11, 2024
1 parent a71cc37 commit 82cbc74
Show file tree
Hide file tree
Showing 4 changed files with 89 additions and 2 deletions.
15 changes: 13 additions & 2 deletions src/config/backends/config-txt.ts
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,11 @@ export class ConfigTxt extends ConfigBackend {
// Split dtoverlays from their params to avoid running into char limits
// and write at the end to prevent overriding the base overlay
if (opts.dtoverlay != null) {
for (const entry of opts.dtoverlay) {
for (let entry of opts.dtoverlay) {
entry = entry.trim();
if (entry.length === 0) {
continue;
}
const [overlay, ...params] = entry.split(',');
confStatements.push(`dtoverlay=${overlay}`);
confStatements.push(...params.map((p) => `dtparam=${p}`));
Expand All @@ -245,9 +249,16 @@ export class ConfigTxt extends ConfigBackend {
public processConfigVarValue(key: string, value: string): string | string[] {
if (isArrayConfig(key)) {
if (!value.startsWith('"')) {
if (key === 'dtoverlay' && value.trim().length === 0) {
return [];
}
return [value];
} else {
return JSON.parse(`[${value}]`);
const res: string[] = JSON.parse(`[${value}]`);
if (key === 'dtoverlay') {
return res.filter((s) => s.trim().length > 0);
}
return res;
}
}
return value;
Expand Down
1 change: 1 addition & 0 deletions src/config/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ export function envToBootConfig(
.mapValues((val, key) =>
configBackend.processConfigVarValue(key, val || ''),
)
.pickBy((val) => !Array.isArray(val) || val.length > 0)
.value();
}

Expand Down
51 changes: 51 additions & 0 deletions test/integration/config/config-txt.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -306,4 +306,55 @@ describe('config/config-txt', () => {

await tfs.restore();
});

it('ignores empty dtoverlay on the target state', async () => {
const tfs = await testfs({
[hostUtils.pathOnBoot('config.txt')]: stripIndent`
enable_uart=1
dtparam=i2c_arm=on
dtparam=spi=on
disable_splash=1
dtparam=audio=on
gpu_mem=16
`,
}).enable();

const configTxt = new ConfigTxt();

await configTxt.setBootConfig({
dtparam: ['i2c=on', 'audio=on'],
dtoverlay: [''],
enable_uart: '1',
avoid_warnings: '1',
gpu_mem: '256',
initramfs: 'initramf.gz 0x00800000',
'hdmi_force_hotplug:1': '1',
});

await expect(
fs.readFile(hostUtils.pathOnBoot('config.txt'), 'utf8'),
).to.eventually.equal(
stripIndent`
dtparam=i2c=on
dtparam=audio=on
enable_uart=1
avoid_warnings=1
gpu_mem=256
initramfs initramf.gz 0x00800000
hdmi_force_hotplug:1=1
` + '\n',
);

// Will try to parse /test/data/mnt/boot/config.txt
await expect(configTxt.getBootConfig()).to.eventually.deep.equal({
dtparam: ['i2c=on', 'audio=on'],
enable_uart: '1',
avoid_warnings: '1',
gpu_mem: '256',
initramfs: 'initramf.gz 0x00800000',
'hdmi_force_hotplug:1': '1',
});

await tfs.restore();
});
});
24 changes: 24 additions & 0 deletions test/integration/device-config.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,30 @@ describe('device-config', () => {
expect(logSpy).to.not.be.called;
});

it('ignores empty dtoverlay when comparing current and target state', async () => {
const current = {
HOST_CONFIG_initramfs: 'initramf.gz 0x00800000',
HOST_CONFIG_dtparam: '"i2c=on","audio=on"',
HOST_CONFIG_foobar: 'baz',
};
const target = {
HOST_CONFIG_initramfs: 'initramf.gz 0x00800000',
HOST_CONFIG_dtparam: '"i2c=on","audio=on"',
HOST_CONFIG_dtoverlay: '',
HOST_CONFIG_foobar: 'baz',
};

expect(
// @ts-expect-error accessing private value
deviceConfig.bootConfigChangeRequired(
configTxtBackend,
current,
target,
),
).to.equal(false);
expect(logSpy).to.not.be.called;
});

it('writes the target config.txt', async () => {
const current = {
HOST_CONFIG_initramfs: 'initramf.gz 0x00800000',
Expand Down

0 comments on commit 82cbc74

Please sign in to comment.