diff --git a/.github/ISSUE_TEMPLATE/app-bug.yml b/.github/ISSUE_TEMPLATE/app-bug.yml new file mode 100644 index 00000000..1f42ec60 --- /dev/null +++ b/.github/ISSUE_TEMPLATE/app-bug.yml @@ -0,0 +1,124 @@ +name: Android app problem +description: The app crashes, shows no locations, will not import a zip, or otherwise misbehaves. +title: "App: " +labels: ["bug", "@app"] +body: + - type: markdown + attributes: + value: | + For the **Android app**. + + The most useful thing you can attach is the log. The app can write one for you — see the + box near the bottom, which explains where the button is and how to make it appear. + + - type: input + id: version + attributes: + label: App version + description: >- + The ⋮ menu in the top right, then **Information**. If you built it yourself, paste + `git rev-parse --short HEAD` instead. + placeholder: "1.0.5" + validations: + required: true + + - type: input + id: device + attributes: + label: Phone and Android version + description: >- + Include the ROM if it is not the manufacturer's — GrapheneOS, LineageOS and similar + restrict things this app relies on, and that has explained real reports here. + placeholder: "Pixel 7, Android 15 — or Pixel 7, GrapheneOS" + validations: + required: true + + - type: dropdown + id: where + attributes: + label: Where does it go wrong? + options: + - Signing in to my Apple account + - Importing the .zip from the exporter + - The map — no locations, or wrong ones + - Location history + - Settings, or the app's own screens + - It crashes + - Something else + validations: + required: true + + - type: textarea + id: what + attributes: + label: What happened + description: >- + What you did, what you expected, and what you got instead. **Screenshots are welcome and + often settle it faster than words** — drag them straight into this box. Anything on screen + that is yours and not the bug's is fine to scribble over. + placeholder: | + I imported the zip, it said 12 tags were added, and the map is empty … + validations: + required: true + + - type: input + id: exporter + attributes: + label: Which exporter made the zip? + description: >- + Only if this is about importing or about missing locations. If the ⋮ menu → + **Information** lists what your tags were imported from, copy it from there; otherwise + whichever version you remember downloading, or leave it blank. + **Do not open the zip to find out.** It holds the keys to your tags, newer ones are + password-protected on purpose, and unpacking it to read a version line is not worth the + risk of what you might then attach. + placeholder: "OpenTagViewer.wizard:1.3.0 — or 1.3.0, or whenever I downloaded it" + + - type: textarea + id: log + attributes: + label: The log + description: | + There are two ways to get one, and the first is easier if it is offered: + + **If the app showed you an error page with an Export logs button**, use that button. It is + the same log, without any of the setting-up below. + + **Otherwise** the button is hidden, because most people never need it: + + 1. **Settings** → turn on **Enable debug data** + 2. Back on the map, open the **⋮** menu in the top right → **Export Logs** + 3. Choose where to save it, and attach that file here + + Either way, make the problem happen *first* and then export — it keeps only the last 500 + lines, so anything older has already scrolled away. + + **Never attach the export zip, or anything unpacked out of it.** It contains the private + keys to your tags: anybody who has it can locate them, and it cannot be un-posted. Nothing + that gets asked here needs it. + + **This is a raw Android log and nothing is removed from it.** Read it before posting and + take out anything you would not put on a public page: your Apple ID, device names, serial + numbers, anything a notification happened to say. Unlike the desktop exporter's Save logs + button, this one does not redact. + + Never paste your Apple ID password, a verification code, or a device passcode. No answer + requires them. + render: shell + + - type: checkboxes + id: privacy + attributes: + label: Before you post + options: + - label: If I attached a log, I have read it and taken out anything personal I did not want public. + required: true + + - type: markdown + attributes: + value: | + This project is not affiliated with Apple or Google. The app talks to Apple's Find My + network using your own account, so a fair number of problems come from Apple changing + something rather than from the app. + + A log usually says which. diff --git a/.github/ISSUE_TEMPLATE/exporter-bug.yml b/.github/ISSUE_TEMPLATE/exporter-bug.yml new file mode 100644 index 00000000..a63c5717 --- /dev/null +++ b/.github/ISSUE_TEMPLATE/exporter-bug.yml @@ -0,0 +1,125 @@ +name: Export tool problem +description: The desktop exporter — the window or the command line — failed, or produced a zip that will not import. +title: "Exporter: " +labels: ["bug", "@exporter-tool"] +body: + - type: markdown + attributes: + value: | + For the **desktop export tool**: the window you download and run on a computer, or the + same thing driven from a terminal. + + Nearly every one of these is answerable from the log, and almost unanswerable without + it. There is a box for it below. + + - type: dropdown + id: how + attributes: + label: How are you running it? + description: >- + The download is the window only. If you are using the command line then you are running + from a copy of the source, which is worth knowing on its own — it means a different + Python and different dependencies from everybody else's. + options: + - The window, from a downloaded release + - The window, run from source + - The command line (always from source) + validations: + required: true + + - type: input + id: version + attributes: + label: Version, or commit + description: >- + **The first line of the log says this**, so if you are attaching one you can copy it from + there. Otherwise: from a downloaded release, the window's title bar. From source, paste + `git rev-parse --short HEAD` — the version in the source is whatever the last release set, + so on a checkout it does not say where you actually are. + placeholder: "1.3.0 — or 1.3.0 (from source, 4713711)" + validations: + required: true + + - type: dropdown + id: os + attributes: + label: Operating system + options: + - Windows + - macOS + - Linux + validations: + required: true + + - type: dropdown + id: route + attributes: + label: Which route? + description: >- + Signing in reads your account over the internet and works anywhere. Reading this Mac's + own files needs a Mac that already has your tags in Find My, and works offline. + options: + - Signing in to iCloud + - Reading this Mac's own Find My files + - Adding a self-generated tag from a key file + - Not sure + validations: + required: true + + - type: textarea + id: what + attributes: + label: What happened + description: What you did, what you expected, and what you got instead. + placeholder: | + I signed in, chose 12 tags, pressed Export, and it said … + validations: + required: true + + - type: textarea + id: log + attributes: + label: The log + description: | + **From the window:** press **Save logs…** at the bottom. It writes a `.txt` and takes the + obvious identifiers out on the way — email address, username, device names, serials. + Attach that file. + + **Never attach the export zip itself, or anything unpacked out of it.** It contains the + private keys to your tags: anybody who has it can locate them, and it cannot be un-posted. + Nothing asked here needs it. The saved log is a different file and is the one to send. + + **From the command line:** re-run with `-vv` and keep the output. + + ``` + uv run python -m exporter.cli -vv 2> exporter-log.txt + ``` + + `2>` matters — the log goes to standard error, so a plain `>` saves an empty file. + + **The command line does not redact anything.** Read `exporter-log.txt` before posting and + take out anything you would not put on a public page: your Apple ID, your computer's user + name inside file paths, device names, serial numbers. Nothing here needs them. + + Never paste an Apple ID password, a verification code, or a device passcode. No answer + requires them and none of them belong in a log. + render: shell + validations: + required: true + + - type: checkboxes + id: privacy + attributes: + label: Before you post + options: + - label: I have read the log and taken out anything personal I did not want public. + required: true + + - type: markdown + attributes: + value: | + This project is not affiliated with Apple. It reads Apple's Find My network using your own + account, so problems usually come from Apple changing something, or from an account shaped + in a way nobody here has seen yet. + + A log that says which is worth more than any amount of description. diff --git a/python/exporter/cli.py b/python/exporter/cli.py index 683c9980..6ec77114 100644 --- a/python/exporter/cli.py +++ b/python/exporter/cli.py @@ -49,7 +49,7 @@ suggested_name, ) from exporter.icloud import Candidate, ExportSourceError -from exporter.version import EXPORT_VIA_CLI, GITHUB_ISSUES_LINK, VERSION +from exporter.version import EXPORT_VIA_CLI, GITHUB_ISSUES_LINK, VERSION, describe_build from opentagviewer_export import ( ExportError, KeyFileError, @@ -241,6 +241,10 @@ def configure_logging(verbosity: int) -> None: format="%(levelname)-8s %(name)s: %(message)s", ) + # Same reason as the wizard's copy: a `-vv` log is what a report attaches, and it said + # nothing about which build wrote it. + logging.getLogger("exporter.version").info("OpenTagViewer exporter %s", describe_build()) + if verbosity >= 2: # **The package, not a list of its subpackages.** This used to name `findmy.cloudkit`, # `findmy.keychain` and `findmy.icloud`, which is every part anybody had needed so far and diff --git a/python/exporter/version.py b/python/exporter/version.py index 7a5364da..b3ad54b7 100644 --- a/python/exporter/version.py +++ b/python/exporter/version.py @@ -19,6 +19,10 @@ from __future__ import annotations +import subprocess +import sys +from pathlib import Path + VERSION = "1.3.0" APP_TITLE = f"OpenTagViewer AirTag Exporter {VERSION}" @@ -43,11 +47,76 @@ report that says which is worth more than one that says "the exporter". """ -GITHUB_ISSUES_LINK = "https://github.com/parawanderer/OpenTagViewer/issues/new" +GITHUB_ISSUES_LINK = ( + "https://github.com/parawanderer/OpenTagViewer/issues/new?template=exporter-bug.yml" +) """ Where to report something neither the user nor this program can fix. Here rather than in `wizard.py`, which is where it used to live alone, so the CLI can say it too without importing tkinter - the same reason `VERSION` is here. Two copies of a URL is exactly the sort of thing that goes stale in one place and is never noticed, because nothing tests a link. + +**Straight at the template, not at a blank form.** The form asks for the version, the route and +the log, which is most of what a report of this needs and almost none of what one arrives with - +and its own front matter carries the labels. Labels can also be put in a URL, as +`?labels=bug,%40exporter-tool`, and that is the worse way round: GitHub applies those only for +somebody with permission to label, which a person reporting a bug generally is not. + +`exporter-bug.yml` is a filename in `.github/ISSUE_TEMPLATE/`, so renaming that file breaks this +link silently - it degrades to a blank issue rather than an error, which is the failure nobody +notices. """ + + +def describe_build() -> str: + """ + What to write in a log so a report says which exporter produced it. + + **`VERSION` alone is not the answer on a checkout.** It is a committed literal, so it says + whatever the last release set for every commit after it - a run from `main` two months into a + release cycle reports the old version perfectly confidently. The commit is the only thing that + identifies such a build, and `exporter-bug.yml` has to ask for it by hand precisely because + nothing in the program said it. + + Three cases, distinguished because they are genuinely different builds: + + - **Frozen**, which is what people download. There is no checkout to ask and `VERSION` is + exactly right, so nothing is spent finding that out. + - **A source tree with git**, where the commit is the truth and the version is a hint. + - **Anything else** - a source zip off a release tag, most likely, where `VERSION` is right + again and there is no commit to name. + """ + if getattr(sys, "frozen", False): + return VERSION + + commit = _commit() + + return f"{VERSION} (from source, {commit})" if commit else VERSION + + +def _commit() -> str | None: + """ + The short commit of the checkout this is running from, or None if that is not a thing. + + **Nothing here is allowed to matter.** It runs while logging is being set up, before anything + the user asked for has started, so every way it can go wrong returns None: no git on PATH, not + a repository, a git that hangs on a network-mounted directory. A version string is not worth + failing a run over, and it is certainly not worth waiting on. + """ + try: + finished = subprocess.run( + ["git", "rev-parse", "--short", "HEAD"], + cwd=Path(__file__).resolve().parent, + capture_output=True, + text=True, + timeout=2, + check=False, + ) + except (OSError, subprocess.SubprocessError): + return None + + if finished.returncode != 0: + return None + + return finished.stdout.strip() or None diff --git a/python/exporter/wizard.py b/python/exporter/wizard.py index 9d4d7ec5..1b7b05b8 100644 --- a/python/exporter/wizard.py +++ b/python/exporter/wizard.py @@ -57,7 +57,13 @@ from findmy.keychain.recovery import RecoveryError from exporter.icloud import Candidate, ExportSourceError -from exporter.version import APP_TITLE, EXPORT_VIA_WIZARD, GITHUB_ISSUES_LINK, VERSION +from exporter.version import ( + APP_TITLE, + EXPORT_VIA_WIZARD, + GITHUB_ISSUES_LINK, + VERSION, + describe_build, +) from opentagviewer_export import ( ExportError, KeyFileError, @@ -1240,6 +1246,12 @@ def configure_logging() -> None: force=True, ) + # **Before the caution, so it is the first line of every run.** A log that arrives attached + # to a report used to say nothing about what produced it, so `exporter-bug.yml` had to ask - + # and the answer people give is the version they remember, which on a checkout is whatever the + # last release set. This is the same gap the app has with `Import.via`. + logging.getLogger("exporter.version").info("OpenTagViewer exporter %s", describe_build()) + _warn_at_the_top_of_the_log() # Anything that escapes Tk's callback handling as well, which otherwise vanishes the same way. diff --git a/python/test/test_build_description.py b/python/test/test_build_description.py new file mode 100644 index 00000000..fe0428b0 --- /dev/null +++ b/python/test/test_build_description.py @@ -0,0 +1,120 @@ +""" +Saying which exporter wrote a log. + +**A log that arrives attached to a bug report used to say nothing about what produced it**, so +`.github/ISSUE_TEMPLATE/exporter-bug.yml` has to ask - and the answer somebody gives is the +version they remember. On a checkout that is whatever the last release set, confidently and +wrongly, because `VERSION` is a committed literal and every commit after a release carries it. + +The same gap the Android app has with `Import.via`: the value is known, and nothing says it. +""" + +from __future__ import annotations + +import logging +import subprocess + +from exporter import cli, version + + +class TestWhatItReports: + def test_a_frozen_build_is_just_the_version(self, monkeypatch): + # What people download. There is no checkout to ask, and `VERSION` is exactly right. + monkeypatch.setattr(version.sys, "frozen", True, raising=False) + + assert version.describe_build() == version.VERSION + + def test_a_frozen_build_does_not_go_looking_for_git(self, monkeypatch): + # Not merely wasteful: `git` in a frozen app's directory is somebody else's repository, + # so an answer from it would be worse than no answer. + monkeypatch.setattr(version.sys, "frozen", True, raising=False) + monkeypatch.setattr(version, "_commit", _never_called) + + version.describe_build() + + def test_a_checkout_names_the_commit(self, monkeypatch): + monkeypatch.setattr(version, "_commit", lambda: "abc1234") + + described = version.describe_build() + + assert version.VERSION in described, "the version is still a useful hint" + assert "abc1234" in described, "and the commit is the part that identifies the build" + + def test_no_git_falls_back_to_the_version(self, monkeypatch): + # A source zip off a release tag, most likely - where VERSION is right again. + monkeypatch.setattr(version, "_commit", lambda: None) + + assert version.describe_build() == version.VERSION + + +class TestItCannotBreakARun: + """ + This runs while logging is being set up, before anything the user asked for has started. + + Every way it can fail has to be a missing suffix rather than a failed export. + """ + + def test_no_git_on_path(self, monkeypatch): + monkeypatch.setattr(subprocess, "run", _raise(FileNotFoundError("git"))) + + assert version._commit() is None + + def test_a_git_that_hangs(self, monkeypatch): + monkeypatch.setattr(subprocess, "run", _raise(subprocess.TimeoutExpired("git", 2))) + + assert version._commit() is None + + def test_not_a_repository(self, monkeypatch): + monkeypatch.setattr(subprocess, "run", _returning(128, "")) + + assert version._commit() is None + + def test_an_empty_answer(self, monkeypatch): + monkeypatch.setattr(subprocess, "run", _returning(0, "\n")) + + assert version._commit() is None + + def test_it_is_asked_of_this_checkout(self, monkeypatch): + # Not of the working directory, which is wherever the user happened to be standing. + seen = {} + + def _record(_args, **kwargs): + seen.update(kwargs) + return _Finished(0, "abc1234") + + monkeypatch.setattr(subprocess, "run", _record) + version._commit() + + assert seen["cwd"].name == "exporter" + assert seen["timeout"], "an unbounded git call can hang a run before it starts" + + +class TestItReachesTheLog: + def test_the_cli_says_it_when_logging_is_turned_up(self, monkeypatch, caplog): + monkeypatch.setattr(version, "_commit", lambda: "abc1234") + + with caplog.at_level(logging.INFO): + cli.configure_logging(2) + + assert any("abc1234" in r.getMessage() for r in caplog.records), caplog.text + + +class _Finished: + def __init__(self, returncode, stdout): + self.returncode = returncode + self.stdout = stdout + + +def _raise(error): + def _boom(*_args, **_kwargs): + raise error + + return _boom + + +def _returning(returncode, stdout): + return lambda *_args, **_kwargs: _Finished(returncode, stdout) + + +def _never_called(): + raise AssertionError("a frozen build must not shell out to git") diff --git a/python/test/test_cli.py b/python/test/test_cli.py index 219a45cd..7bc33b2c 100644 --- a/python/test/test_cli.py +++ b/python/test/test_cli.py @@ -564,3 +564,31 @@ def test_a_mistake_the_user_can_fix_does_not_ask_for_a_bug_report(self, monkeypa assert cli.main([]) == 1 assert GITHUB_ISSUES_LINK not in capsys.readouterr().err + + def test_the_template_it_links_to_exists(self): + """ + The link names a file in `.github/ISSUE_TEMPLATE/`, and a rename breaks it in silence. + + GitHub does not error on an unknown `?template=`; it drops the reporter on a blank issue + with none of the questions and none of the labels. So the failure is a slightly worse bug + report, months later, and nothing anywhere says why. + """ + name = GITHUB_ISSUES_LINK.partition("template=")[2] + assert name, "the issues link no longer names a template" + + template = Path(__file__).resolve().parents[2] / ".github" / "ISSUE_TEMPLATE" / name + assert template.is_file(), f"{GITHUB_ISSUES_LINK} points at a template that is not there" + + def test_the_template_carries_the_labels_itself(self): + # Rather than the URL carrying them. GitHub applies `?labels=` only for somebody with + # permission to label, which a person reporting a bug generally is not - so a URL that + # looks like it labels things would quietly not. + import yaml # noqa: PLC0415 - only this test needs it + + name = GITHUB_ISSUES_LINK.partition("template=")[2] + template = Path(__file__).resolve().parents[2] / ".github" / "ISSUE_TEMPLATE" / name + front = yaml.safe_load(template.read_text(encoding="utf-8")) + + assert "bug" in front["labels"] + assert "@exporter-tool" in front["labels"] + assert "labels=" not in GITHUB_ISSUES_LINK