Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

desktop: stop renaming desktop file #5150

Open
wants to merge 1 commit into
base: feature/desktop-files
Choose a base branch
from
Open

desktop: stop renaming desktop file #5150

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 10 additions & 4 deletions snapcraft/parts/desktop_file.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ class DesktopFile:

:param snap_name: The snap package name.
:param app_name: The name of the app using the desktop file.
:param filename: The desktop file name.
:param filename: The desktop file path.
:param prime_dir: The prime directory path.

:raises DesktopFileError: If the desktop file does not exist.
Expand Down Expand Up @@ -116,9 +116,15 @@ def write(self, *, gui_dir: Path, icon_path: Optional[str] = None) -> None:

gui_dir.mkdir(parents=True, exist_ok=True)

# Rename the desktop file to match the app name. This will help
# unity8 associate them (https://launchpad.net/bugs/1659330).
target = gui_dir / f"{self._app_name}.desktop"
desktop_filename = os.path.basename(self._filename)

# Stop renaming the desktop file. From snapd 2.66 onwards,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this comment is very good, but I think we should drop the first sentence - it's awkward and potentially confusing to refer to something the code no longer does

# users can declare custom desktop file names which snapd will not rename
# in the format of {SNAP_NAME}_{APP_NAME}.desktop
# https://snapcraft.io/docs/desktop-interface
target = gui_dir / desktop_filename
if not (desktop_filename.endswith(".desktop")):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can replace this code with a with_suffix(".desktop") call

>>> Path("file").with_suffix(".desktop")
PosixPath('file.desktop')
>>> Path("file.desktop").with_suffix(".desktop")
PosixPath('file.desktop')

target = gui_dir / f"{desktop_filename}.desktop"

if target.exists():
# Unlikely. A desktop file in meta/gui/ already existed for
Expand Down
10 changes: 10 additions & 0 deletions tests/conftest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
# along with this program. If not, see <http://www.gnu.org/licenses/>.

import os
from pathlib import Path

import keyring
import pytest
Expand Down Expand Up @@ -52,6 +53,15 @@ def new_dir(tmp_path):
os.chdir(cwd)


@pytest.fixture
def prime_dir(new_dir):
"""Create a subdirectory structure 'new_dir/meta/gui'."""

prime_dir = Path(f"{new_dir}/meta/gui")

yield prime_dir
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
yield prime_dir
return prime_dir



@pytest.fixture
def memory_keyring():
"""In memory keyring backend for testing."""
Expand Down
4 changes: 2 additions & 2 deletions tests/spread/core24/appstream-desktop/task.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ execute: |
exit 1
fi

if ! diff -U10 prime/meta/gui/appstream-desktop.desktop expected_appstream-desktop.desktop; then
echo "The formatting for appstream-desktop.desktop is incorrect"
if ! diff -U10 prime/meta/gui/io.snapcraft.appstream.desktop expected_appstream-desktop.desktop; then
echo "The formatting for io.snapcraft.appstream.desktop is incorrect"
exit 1
fi
4 changes: 2 additions & 2 deletions tests/spread/general/appstream-desktop/task.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ execute: |

expected_desktop="$PWD/../../appstream-desktop/expected_appstream-desktop.desktop"

if ! diff -U10 prime/meta/gui/appstream-desktop.desktop "$expected_desktop"; then
echo "The formatting for appstream-desktop.desktop is incorrect"
if ! diff -U10 prime/meta/gui/io.snapcraft.appstream.desktop "$expected_desktop"; then
echo "The formatting for io.snapcraft.appstream.desktop is incorrect"
exit 1
fi
4 changes: 2 additions & 2 deletions tests/spread/general/appstream-icon-from-desktop/task.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ execute: |

expected_desktop="$PWD/../../appstream-desktop/expected_appstream-desktop.desktop"

if ! diff -U10 prime/meta/gui/appstream-desktop.desktop "$expected_desktop"; then
echo "The formatting for appstream-desktop.desktop is incorrect"
if ! diff -U10 prime/meta/gui/io.snapcraft.appstream.desktop "$expected_desktop"; then
echo "The formatting for io.snapcraft.appstream.desktop is incorrect"
exit 1
fi
4 changes: 2 additions & 2 deletions tests/spread/general/appstream-remote-gfx/task.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ execute: |

expected_desktop="$PWD/../../appstream-remote-gfx/expected_appstream-desktop.desktop"

if ! diff -U10 prime/meta/gui/appstream-desktop.desktop "$expected_desktop"; then
echo "The formatting for appstream-desktop.desktop is incorrect"
if ! diff -U10 prime/meta/gui/io.snapcraft.appstream.desktop "$expected_desktop"; then
echo "The formatting for io.snapcraft.appstream.desktop is incorrect"
exit 1
fi

Expand Down
48 changes: 35 additions & 13 deletions tests/unit/parts/test_desktop_file.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.

from pathlib import Path
from textwrap import dedent

import pytest
Expand All @@ -39,7 +38,9 @@ class TestDesktopExec:
("bar", "--arg", "foo.bar --arg"),
],
)
def test_generate_desktop_file(self, new_dir, app_name, app_args, expected_exec):
def test_generate_desktop_file(
self, new_dir, prime_dir, app_name, app_args, expected_exec
):
snap_name = "foo"

desktop_file_path = new_dir / "app.desktop"
Expand All @@ -53,9 +54,9 @@ def test_generate_desktop_file(self, new_dir, app_name, app_args, expected_exec)
filename=desktop_file_path,
prime_dir=new_dir.as_posix(),
)
d.write(gui_dir=Path())
d.write(gui_dir=prime_dir)

expected_desktop_file = new_dir / f"{app_name}.desktop"
expected_desktop_file = prime_dir / "app.desktop"
assert expected_desktop_file.exists()
with expected_desktop_file.open() as desktop_file:
assert desktop_file.read() == dedent(
Expand Down Expand Up @@ -83,7 +84,9 @@ class TestDesktopIcon:
("foo", None, "foo"),
],
)
def test_generate_desktop_file(self, new_dir, icon, icon_path, expected_icon):
def test_generate_desktop_file(
self, new_dir, prime_dir, icon, icon_path, expected_icon
):
snap_name = app_name = "foo"

desktop_file_path = new_dir / "app.desktop"
Expand All @@ -101,14 +104,14 @@ def test_generate_desktop_file(self, new_dir, icon, icon_path, expected_icon):
filename=desktop_file_path,
prime_dir=new_dir,
)
d.write(gui_dir=Path())
d.write(gui_dir=prime_dir)

if icon_path is not None:
d.write(icon_path=icon_path, gui_dir=Path())
d.write(icon_path=icon_path, gui_dir=prime_dir)
else:
d.write(gui_dir=Path())
d.write(gui_dir=prime_dir)

expected_desktop_file = new_dir / f"{app_name}.desktop"
expected_desktop_file = prime_dir / "app.desktop"
assert expected_desktop_file.exists()
with expected_desktop_file.open() as desktop_file:
assert (
Expand Down Expand Up @@ -137,7 +140,7 @@ def test_generate_desktop_file(self, new_dir, icon, icon_path, expected_icon):
],
)
def test_generate_desktop_file_multisection(
self, new_dir, icon, icon_path, expected_icon
self, new_dir, prime_dir, icon, icon_path, expected_icon
):
snap_name = app_name = "foo"

Expand All @@ -161,11 +164,11 @@ def test_generate_desktop_file_multisection(
)

if icon_path is not None:
d.write(icon_path=icon_path, gui_dir=Path())
d.write(icon_path=icon_path, gui_dir=prime_dir)
else:
d.write(gui_dir=Path())
d.write(gui_dir=prime_dir)

expected_desktop_file = new_dir / f"{app_name}.desktop"
expected_desktop_file = prime_dir / "app.desktop"
assert expected_desktop_file.exists()
with expected_desktop_file.open() as desktop_file:
assert desktop_file.read() == dedent(
Expand Down Expand Up @@ -223,3 +226,22 @@ def test_missing_exec_entry(new_dir):

with pytest.raises(errors.DesktopFileError):
d.write(gui_dir=new_dir)


def test_desktop_without_extension(new_dir, prime_dir):
desktop_file_path = new_dir / "app"
app_args = ""
with desktop_file_path.open("w") as desktop_file:
print("[Desktop Entry]", file=desktop_file)
print(f"Exec={' '.join(['in-snap-exe', app_args])}", file=desktop_file)

d = DesktopFile(
snap_name="snap",
app_name="app1",
filename=desktop_file_path,
prime_dir=new_dir.as_posix(),
)
d.write(gui_dir=prime_dir)

expected_desktop_file = prime_dir / "app.desktop"
assert expected_desktop_file.exists()
6 changes: 3 additions & 3 deletions tests/unit/parts/test_setup_assets.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ def test_setup_assets_happy(self, desktop_file, yaml_data, new_dir):
)

# desktop file should be in meta/gui and named after app
desktop_path = Path("prime/meta/gui/app1.desktop")
desktop_path = Path("prime/meta/gui/test.desktop")
assert desktop_path.is_file()

# desktop file content should make icon relative to ${SNAP}
Expand Down Expand Up @@ -379,7 +379,7 @@ def test_setup_assets_icon_in_assets_dir(self, desktop_file, yaml_data, new_dir)
)

# desktop file should be in meta/gui and named after app
desktop_path = Path("prime/meta/gui/app1.desktop")
desktop_path = Path("prime/meta/gui/test.desktop")
assert desktop_path.is_file()

# desktop file content should make icon relative to ${SNAP}
Expand Down Expand Up @@ -449,7 +449,7 @@ def test_setup_assets_remote_icon(self, desktop_file, yaml_data, new_dir):
)

# desktop file should be in meta/gui and named after app
desktop_path = Path("prime/meta/gui/app1.desktop")
desktop_path = Path("prime/meta/gui/test.desktop")
assert desktop_path.is_file()

# desktop file content should make icon relative to ${SNAP}
Expand Down
Loading