Skip to content

Commit a83e6a8

Browse files
committed
fix: improve desktop file path resolution in content command
Enhanced the content command to properly resolve desktop file paths by checking both default and overlay shared directories. Previously, desktop files were always treated as regular files without considering the actual exported locations. Key changes: 1. Added resolveDesktopFileExportPath method to OSTreeRepo that checks both default and overlay directories 2. Modified content command to use this method for desktop files while keeping regular files unchanged 3. Improved content filtering logic to only show existing exported files 4. Added comprehensive unit tests covering both default and overlay directory scenarios The fix ensures that desktop files are correctly resolved to their actual exported locations, preventing issues where desktop files might not be found if they exist in overlay directories but not in default directories. Influence: 1. Test content command with applications that have desktop files in default shared directory 2. Test content command with applications that have desktop files only in overlay directory 3. Verify both cases work correctly and show proper paths 4. Test with applications that have no desktop files to ensure regular file handling remains unchanged 5. Verify the output only includes existing files after path resolution
1 parent 1b35461 commit a83e6a8

5 files changed

Lines changed: 298 additions & 21 deletions

File tree

libs/linglong/src/linglong/cli/cli.cpp

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1668,31 +1668,40 @@ int Cli::content(const ContentOptions &options)
16681668
return -1;
16691669
}
16701670

1671-
QDir entriesDir((layer->path() / "entries/share").c_str());
1671+
QDir entriesDir((layer->path() / "entries").c_str());
16721672
if (!entriesDir.exists()) {
16731673
this->printer.printErr(LINGLONG_ERR("no entries found").value());
16741674
return -1;
16751675
}
16761676

1677+
const auto preferLibSystemdUser = QFileInfo(entriesDir.filePath("lib/systemd/user")).exists();
1678+
16771679
QDirIterator it(entriesDir.absolutePath(),
16781680
QDir::AllEntries | QDir::NoDot | QDir::NoDotDot | QDir::System,
16791681
QDirIterator::Subdirectories);
16801682
while (it.hasNext()) {
16811683
it.next();
1682-
contents.append(it.fileInfo().absoluteFilePath());
1684+
const auto entryPath = it.fileInfo().absoluteFilePath();
1685+
const auto relativePath =
1686+
std::filesystem::path(entriesDir.relativeFilePath(entryPath).toStdString());
1687+
const auto exportPath =
1688+
this->repository.resolveEntryExportPath(relativePath, preferLibSystemdUser);
1689+
if (!exportPath.empty()) {
1690+
contents.append(QString::fromStdString(exportPath.string()));
1691+
}
16831692
}
1684-
// replace $LINGLONG_ROOT/layers/appid/verison/arch/module/entries to ${LINGLONG_ROOT}/entires
1685-
contents.replaceInStrings(entriesDir.absolutePath(), QString(LINGLONG_ROOT) + "/entries/share");
16861693

16871694
// only show the contents which are exported
1688-
for (int pos = 0; pos < contents.size(); ++pos) {
1689-
QFileInfo info(contents.at(pos));
1690-
if (!info.exists()) {
1691-
contents.removeAt(pos);
1695+
QStringList exportedContents{};
1696+
for (const auto &content : std::as_const(contents)) {
1697+
QFileInfo info(content);
1698+
if (!info.exists() || info.isDir()) {
1699+
continue;
16921700
}
1701+
exportedContents.append(content);
16931702
}
16941703

1695-
this->printer.printContent(contents);
1704+
this->printer.printContent(exportedContents);
16961705
return 0;
16971706
}
16981707

libs/linglong/src/linglong/repo/ostree_repo.cpp

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3054,6 +3054,67 @@ QDir OSTreeRepo::getDefaultSharedDir() const noexcept
30543054
return this->repoDir.absoluteFilePath("entries/share");
30553055
}
30563056

3057+
std::filesystem::path OSTreeRepo::resolveEntryExportPath(const std::filesystem::path &relativePath,
3058+
bool preferLibSystemdUser) const noexcept
3059+
{
3060+
auto relative = relativePath.generic_string();
3061+
if (relative.empty()) {
3062+
return {};
3063+
}
3064+
3065+
const auto repoDirPath = std::filesystem::path(this->repoDir.absolutePath().toStdString());
3066+
const auto entriesDir = repoDirPath / "entries";
3067+
3068+
if (common::strings::starts_with(relative, "share/applications/")
3069+
&& common::strings::ends_with(relative, ".desktop")) {
3070+
return this->resolveDesktopFileExportPath(
3071+
std::filesystem::path(relative).lexically_relative(std::filesystem::path{ "share" }));
3072+
}
3073+
3074+
// exportEntries() will export systemd user units to entries/lib/systemd/user:
3075+
// 1. Prefer entries/lib/systemd/user when the app ships it directly.
3076+
// 2. Fall back to entries/share/systemd/user only for legacy apps, but still map the
3077+
// resulting visible path to entries/lib/systemd/user.
3078+
// Returning an empty path here skips the legacy share path when lib/systemd/user exists,
3079+
// so `ll-cli content` does not emit the same exported target twice.
3080+
constexpr std::string_view shareSystemdUser = "share/systemd/user";
3081+
if (relative == shareSystemdUser
3082+
|| common::strings::starts_with(relative, "share/systemd/user/")) {
3083+
if (preferLibSystemdUser) {
3084+
return {};
3085+
}
3086+
3087+
auto suffix = relative.substr(shareSystemdUser.size());
3088+
return entriesDir / std::filesystem::path("lib/systemd/user" + suffix);
3089+
}
3090+
3091+
constexpr std::string_view libSystemdUser = "lib/systemd/user";
3092+
if (relative == libSystemdUser || common::strings::starts_with(relative, "lib/systemd/user/")) {
3093+
return entriesDir / relativePath;
3094+
}
3095+
3096+
return entriesDir / relativePath;
3097+
}
3098+
3099+
std::filesystem::path
3100+
OSTreeRepo::resolveDesktopFileExportPath(const std::filesystem::path &relativePath) const noexcept
3101+
{
3102+
const auto defaultDesktopPath =
3103+
std::filesystem::path(this->getDefaultSharedDir().absolutePath().toStdString())
3104+
/ relativePath;
3105+
const auto overlayDesktopPath =
3106+
std::filesystem::path(this->getOverlayShareDir().absolutePath().toStdString()) / relativePath;
3107+
if (overlayDesktopPath != defaultDesktopPath && std::filesystem::exists(overlayDesktopPath)) {
3108+
return overlayDesktopPath;
3109+
}
3110+
3111+
if (std::filesystem::exists(defaultDesktopPath)) {
3112+
return defaultDesktopPath;
3113+
}
3114+
3115+
return overlayDesktopPath;
3116+
}
3117+
30573118
QDir OSTreeRepo::getOverlayShareDir() const noexcept
30583119
{
30593120
return this->repoDir.absoluteFilePath("entries/" LINGLONG_EXPORT_PATH);

libs/linglong/src/linglong/repo/ostree_repo.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323

2424
#include <QDir>
2525

26+
#include <filesystem>
2627
#include <memory>
2728
#include <optional>
2829
#include <string>
@@ -190,6 +191,10 @@ class OSTreeRepo : public QObject
190191
std::string module = "binary",
191192
const std::optional<std::string> &subRef = std::nullopt) const noexcept;
192193
utils::error::Result<void> fixExportAllEntries() noexcept;
194+
[[nodiscard]] std::filesystem::path resolveEntryExportPath(
195+
const std::filesystem::path &relativePath, bool preferLibSystemdUser = false) const noexcept;
196+
[[nodiscard]] std::filesystem::path
197+
resolveDesktopFileExportPath(const std::filesystem::path &relativePath) const noexcept;
193198

194199
virtual std::unique_ptr<ClientAPIWrapper> createClientV2(const std::string &url) const
195200
{

libs/linglong/tests/ll-tests/src/linglong/cli/cli_test.cpp

Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,9 @@
1414
#include "linglong/cli/printer.h"
1515
#include "ocppi/cli/crun/Crun.hpp"
1616

17+
#include <filesystem>
18+
#include <fstream>
19+
1720
using namespace linglong;
1821
using ::testing::_;
1922
using ::testing::ElementsAre;
@@ -32,10 +35,27 @@ class MockRepo : public repo::OSTreeRepo
3235
{
3336
}
3437

38+
QDir defaultSharedDir() const noexcept { return this->getDefaultSharedDir(); }
39+
40+
QDir overlaySharedDir() const noexcept { return this->getOverlayShareDir(); }
41+
3542
MOCK_METHOD(utils::error::Result<std::vector<api::types::v1::PackageInfoV2>>,
3643
listLocal,
3744
(),
3845
(override, const, noexcept));
46+
MOCK_METHOD(utils::error::Result<package::Reference>,
47+
clearReference,
48+
(const package::FuzzyReference &fuzzyRef,
49+
const repo::clearReferenceOption &opts,
50+
const std::string &module,
51+
const std::optional<std::string> &repo),
52+
(override, const, noexcept));
53+
MOCK_METHOD(utils::error::Result<api::types::v1::RepositoryCacheLayersItem>,
54+
getLayerItem,
55+
(const package::Reference &ref,
56+
std::string module,
57+
const std::optional<std::string> &subRef),
58+
(override, const, noexcept));
3959
MOCK_METHOD(utils::error::Result<package::ReferenceWithRepo>,
4060
latestRemoteReference,
4161
(const package::FuzzyReference &fuzzyRef),
@@ -49,6 +69,7 @@ class MockPrinter : public cli::CLIPrinter
4969
printUpgradeList,
5070
(std::vector<api::types::v1::UpgradeListResult> &),
5171
(override));
72+
MOCK_METHOD(void, printContent, (const QStringList &filePaths), (override));
5273
};
5374

5475
class CliTest : public ::testing::Test
@@ -237,4 +258,145 @@ TEST_F(CliTest, listUpgradableNoUpgrade)
237258
cli->list(cli::ListOptions{ .showUpgradeList = true });
238259
}
239260

261+
TEST_F(CliTest, contentPreferDesktopFromDefaultSharedDir)
262+
{
263+
auto ref = package::Reference::parse("main:org.example.app/1.0.0/x86_64");
264+
ASSERT_TRUE(ref.has_value());
265+
266+
const std::string commit = "test-commit-default";
267+
const auto layerEntriesDir =
268+
tempDir->path() / "layers" / commit / "entries" / "share" / "applications";
269+
const auto defaultDesktopPath =
270+
std::filesystem::path(repo->defaultSharedDir().absolutePath().toStdString()) / "applications"
271+
/ "org.example.app.desktop";
272+
273+
std::filesystem::create_directories(layerEntriesDir);
274+
std::filesystem::create_directories(defaultDesktopPath.parent_path());
275+
std::ofstream(layerEntriesDir / "org.example.app.desktop") << "Test desktop content";
276+
std::ofstream(defaultDesktopPath) << "Test desktop content in default";
277+
278+
api::types::v1::RepositoryCacheLayersItem layerItem;
279+
layerItem.commit = commit;
280+
layerItem.info.kind = "app";
281+
282+
EXPECT_CALL(*repo, clearReference(_, _, _, _)).WillOnce(Return(*ref));
283+
EXPECT_CALL(*repo, getLayerItem(_, _, _))
284+
.WillRepeatedly(
285+
[layerItem](const package::Reference &, std::string, const std::optional<std::string> &)
286+
-> utils::error::Result<api::types::v1::RepositoryCacheLayersItem> {
287+
return layerItem;
288+
});
289+
EXPECT_CALL(*printer,
290+
printContent(ElementsAre(QString::fromStdString(defaultDesktopPath.string()))))
291+
.WillOnce(Return());
292+
293+
EXPECT_EQ(cli->content(cli::ContentOptions{ .appid = "org.example.app" }), 0);
294+
}
295+
296+
TEST_F(CliTest, contentFallbackDesktopToOverlaySharedDir)
297+
{
298+
auto ref = package::Reference::parse("main:org.example.app/1.0.0/x86_64");
299+
ASSERT_TRUE(ref.has_value());
300+
301+
const std::string commit = "test-commit-overlay";
302+
const auto layerEntriesDir =
303+
tempDir->path() / "layers" / commit / "entries" / "share" / "applications";
304+
const auto overlayDesktopPath =
305+
std::filesystem::path(repo->overlaySharedDir().absolutePath().toStdString()) / "applications"
306+
/ "org.example.app.desktop";
307+
308+
std::filesystem::create_directories(layerEntriesDir);
309+
std::filesystem::create_directories(overlayDesktopPath.parent_path());
310+
std::ofstream(layerEntriesDir / "org.example.app.desktop") << "Test desktop content";
311+
std::ofstream(overlayDesktopPath) << "Test desktop content in overlay";
312+
313+
api::types::v1::RepositoryCacheLayersItem layerItem;
314+
layerItem.commit = commit;
315+
layerItem.info.kind = "app";
316+
317+
EXPECT_CALL(*repo, clearReference(_, _, _, _)).WillOnce(Return(*ref));
318+
EXPECT_CALL(*repo, getLayerItem(_, _, _))
319+
.WillRepeatedly(
320+
[layerItem](const package::Reference &, std::string, const std::optional<std::string> &)
321+
-> utils::error::Result<api::types::v1::RepositoryCacheLayersItem> {
322+
return layerItem;
323+
});
324+
EXPECT_CALL(*printer,
325+
printContent(ElementsAre(QString::fromStdString(overlayDesktopPath.string()))))
326+
.WillOnce(Return());
327+
328+
EXPECT_EQ(cli->content(cli::ContentOptions{ .appid = "org.example.app" }), 0);
329+
}
330+
331+
TEST_F(CliTest, contentMapsLegacySystemdUserPathToExportedLibPath)
332+
{
333+
auto ref = package::Reference::parse("main:org.example.app/1.0.0/x86_64");
334+
ASSERT_TRUE(ref.has_value());
335+
336+
const std::string commit = "test-commit-systemd-legacy";
337+
const auto layerEntriesDir =
338+
tempDir->path() / "layers" / commit / "entries" / "share" / "systemd" / "user";
339+
const auto exportedDir = tempDir->path() / "entries" / "lib" / "systemd" / "user";
340+
const auto exportedFile = exportedDir / "org.example.app.service";
341+
342+
std::filesystem::create_directories(layerEntriesDir);
343+
std::filesystem::create_directories(exportedDir);
344+
std::ofstream(layerEntriesDir / "org.example.app.service")
345+
<< "[Service]\nExecStart=/bin/true\n";
346+
std::ofstream(exportedFile) << "[Service]\nExecStart=/bin/true\n";
347+
348+
api::types::v1::RepositoryCacheLayersItem layerItem;
349+
layerItem.commit = commit;
350+
layerItem.info.kind = "app";
351+
352+
EXPECT_CALL(*repo, clearReference(_, _, _, _)).WillOnce(Return(*ref));
353+
EXPECT_CALL(*repo, getLayerItem(_, _, _))
354+
.WillRepeatedly(
355+
[layerItem](const package::Reference &, std::string, const std::optional<std::string> &)
356+
-> utils::error::Result<api::types::v1::RepositoryCacheLayersItem> {
357+
return layerItem;
358+
});
359+
EXPECT_CALL(*printer, printContent(ElementsAre(QString::fromStdString(exportedFile.string()))))
360+
.WillOnce(Return());
361+
362+
EXPECT_EQ(cli->content(cli::ContentOptions{ .appid = "org.example.app" }), 0);
363+
}
364+
365+
TEST_F(CliTest, contentPrefersLibSystemdUserOverLegacySharePath)
366+
{
367+
auto ref = package::Reference::parse("main:org.example.app/1.0.0/x86_64");
368+
ASSERT_TRUE(ref.has_value());
369+
370+
const std::string commit = "test-commit-systemd-lib";
371+
const auto layerLibDir =
372+
tempDir->path() / "layers" / commit / "entries" / "lib" / "systemd" / "user";
373+
const auto layerLegacyDir =
374+
tempDir->path() / "layers" / commit / "entries" / "share" / "systemd" / "user";
375+
const auto exportedDir = tempDir->path() / "entries" / "lib" / "systemd" / "user";
376+
const auto exportedFile = exportedDir / "org.example.app.service";
377+
378+
std::filesystem::create_directories(layerLibDir);
379+
std::filesystem::create_directories(layerLegacyDir);
380+
std::filesystem::create_directories(exportedDir);
381+
std::ofstream(layerLibDir / "org.example.app.service") << "[Service]\nExecStart=/bin/true\n";
382+
std::ofstream(layerLegacyDir / "org.example.app.service") << "[Service]\nExecStart=/bin/true\n";
383+
std::ofstream(exportedFile) << "[Service]\nExecStart=/bin/true\n";
384+
385+
api::types::v1::RepositoryCacheLayersItem layerItem;
386+
layerItem.commit = commit;
387+
layerItem.info.kind = "app";
388+
389+
EXPECT_CALL(*repo, clearReference(_, _, _, _)).WillOnce(Return(*ref));
390+
EXPECT_CALL(*repo, getLayerItem(_, _, _))
391+
.WillRepeatedly(
392+
[layerItem](const package::Reference &, std::string, const std::optional<std::string> &)
393+
-> utils::error::Result<api::types::v1::RepositoryCacheLayersItem> {
394+
return layerItem;
395+
});
396+
EXPECT_CALL(*printer, printContent(ElementsAre(QString::fromStdString(exportedFile.string()))))
397+
.WillOnce(Return());
398+
399+
EXPECT_EQ(cli->content(cli::ContentOptions{ .appid = "org.example.app" }), 0);
400+
}
401+
240402
} // namespace

0 commit comments

Comments
 (0)