Skip to content

FIxes #3043 Added Non Interactive Install script - #3063

Merged
bbedward merged 2 commits into
AvengeMedia:masterfrom
JDKamalakar:master
Aug 28, 2026
Merged

FIxes #3043 Added Non Interactive Install script#3063
bbedward merged 2 commits into
AvengeMedia:masterfrom
JDKamalakar:master

Conversation

@JDKamalakar

Copy link
Copy Markdown
Contributor

Description

This change fixes #3043 making it sure that user can set all the flags wanted & then run install without any interaction

Type of change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that changes existing behavior)
  • Refactor / internal cleanup
  • Documentation
  • Other

Related issues

Fixes #3043

Screenshots / video

No user facing change only adds flags to the install script tested on my forked repo.

Checklist

  • My code follows the conventions in CONTRIBUTING.md
  • I have tested my changes locally
  • New user-facing strings are wrapped in I18n.tr() with translator context, reusing existing terms where possible
  • Go changes: ran make fmt, added/updated tests, make test passes, and go mod tidy is clean
  • QML changes: ran make lint-qml with no new warnings
  • I have opened a corresponding pull request in dlx-docs to document any new behaviors: https://github.com/AvengeMedia/DankLinux-Docs

Note: I have no experience of go language as well as entire change is made by AI :: Gemini basically Docs are not created but will create as soon as someone with the knowledge of GO lang conforms the changes are correct or is there any change.

Write Now there is install_script_flags.md flie for the list of flags but can be removed after approval & changes on docs are done.

Comment thread core/install.sh Outdated
Comment thread core/README.md Outdated
Comment thread install_script_flags.md Outdated
Comment thread core/internal/headless/runner.go Outdated
Comment thread core/internal/headless/runner.go Outdated
Comment thread core/internal/headless/runner.go Outdated
Comment thread core/internal/headless/runner.go Outdated
Comment thread core/cmd/dankinstall/main.go Outdated
Comment thread core/internal/headless/runner.go Outdated
Comment thread core/cmd/dankinstall/main.go Outdated
@JDKamalakar
JDKamalakar requested a review from bbedward August 16, 2026 15:05
@bbedward

Copy link
Copy Markdown
Collaborator

/claude review

Comment thread core/install.sh
# Execute the installer
printf "%bRunning installer...%b\n" "$GREEN" "$NC"
./installer
./installer "$@"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Forwarding args makes a non-zero installer exit a routine outcome (missing --yes, bad --compositor/--privesc value, unknown --git-deps name). With set -e at the top of the script, that aborts here and the cleanup on lines 85-86 never runs, so the ~15MB extracted installer and its temp dir are left behind in /tmp on every failed headless invocation. Previously the piped script always ran the TUI, which exits 0.

A trap right after TEMP_DIR is created makes cleanup unconditional and preserves the installer's exit code:

TEMP_DIR=$(mktemp -d)
trap 'rm -rf "$TEMP_DIR"' EXIT

Comment on lines +321 to +326
markGit := func(dep *deps.Dependency) {
if dep.Variant == deps.VariantStable && dep.Status == deps.StatusInstalled {
reinstallItems[dep.Name] = true
}
dep.Variant = deps.VariantGit
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

CanToggle is not equivalent to "has a git variant", so this gates on the wrong thing. hyprland is created with CanToggle: true for every distro (distros/base.go:314), but Arch's mapping explicitly discards the variant:

func (a *ArchDistribution) getHyprlandMapping(_ deps.PackageVariant) PackageMapping {
	return PackageMapping{Name: "hyprland", Repository: RepoTypeSystem}
}

So --git-deps hyprland (or --git) on Arch passes the CanToggle check, marks hyprland for reinstall here, and then categorizePackages resolves it back to the plain hyprland package — the user is told they're getting git and instead gets a pointless reinstall of the stable package they already have. Headless had no reinstall path before this PR, so the wasted rebuild is new.

Resolving the mapping and only marking a dep when the git and stable package names actually differ would keep --git-deps hyprland from lying, and would let the unknown-variant case return an error instead of silently installing stable.

@claude

claude Bot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Claude review

Two minor issues; the flag plumbing, --all-features guard and --privesc handling all look correct, and the earlier review comments appear addressed.

  • Temp dir is leaked on any failed headless run now that args are forwarded (set -e skips the cleanup), core/install.sh:82
  • CanToggle is used as "has a git variant", but Arch discards the variant for hyprland, so --git/--git-deps hyprland silently reinstalls the stable package, core/internal/headless/runner.go:321

Checked: git-variant/reinstall interaction with categorizePackages and the config deployer, --all-features vs --exclude-deps/dedicated flag ordering, --privesc precedence over $DMS_PRIVESC, headless-only flag rejection, README accuracy against the flag set, and the new tests. Model: claude-opus-5.

@bbedward

Copy link
Copy Markdown
Collaborator

/claude review

@claude

claude Bot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Claude review

No issues found. Both findings from the previous review are fixed, and the rest of the change holds up.

Checked: install.sh now cleans up via an EXIT trap so a non-zero ./installer "$@" under set -e no longer leaks the temp dir, and the release URLs point back at AvengeMedia; gitVariantChecker resolves stable vs. all-git PackageMappings and compares them, so --git/--git-deps no longer trust CanToggle alone (Arch/Fedora hyprland and every Gentoo dep correctly report no git variant, and --git-deps errors instead of silently installing stable); PackageMapping is comparable so != is a valid check; applyGitVariants mutates through &dependencies[i] so variants reach categorizePackages, and its reinstallItems is safe because every distro checks disabledFlags before reinstallFlags and the config deployer ignores the reinstall map entirely (no unexpected config overwrites); --all-features only skips the "disable non-required" loop, and detectOptionalPackage is the sole source of Required: false (danksearch/dankcalendar/dms-greeter), so it does not over-enable, and --exclude-deps still wins; --privesc validates before SetTool and takes precedence over $DMS_PRIVESC; new flags are all listed in the TUI rejection set with matching names; GetPackageMappingWithVariants was already implemented on all seven distros, so the interface addition compiles and there are no mocks or test fakes to regenerate; README table matches the flag set. Model: claude-opus-5.

@bbedward
bbedward merged commit f086779 into AvengeMedia:master Aug 28, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Non Interactive DMS Installer

2 participants