make-desktopitem: refactoring, documentation and improvement

[piegamesde]

Motivation for this change

• New parameter extraDesktopEntries to easily add some less usual entries to the desktop file
• Rewrite of the core logic. Instead of a key-value-list, use an attribute set with nullable values to make it overridable
• Added some comments
• Some cosmetic/readability code refactors
  • I didn't like the doubly nested strings around the fileValidation

Things done

• Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
• Built on platform(s)
  • NixOS
  • macOS
  • other Linux distributions
• Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
• Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
• Tested execution of all binary files (usually in ./result/bin/)
• Determined the impact on package closure size (by running nix path-info -S before and after)
• Ensured that relevant documentation is up to date
• Fits CONTRIBUTING.md.

[mweinelt]

Result of nixpkgs-review pr 91790 1

5 packages marked as broken and skipped:

- eagle7
- invoice2data
- pdfdiff
- pgadmin
- xpdf

24 packages failed to build:

- citrix_workspace (citrix_workspace_unwrapped ,citrix_workspace_unwrapped_20_04_0)
- citrix_workspace_19_10_0 (citrix_workspace_unwrapped_19_10_0)
- citrix_workspace_19_12_0 (citrix_workspace_unwrapped_19_12_0)
- citrix_workspace_19_6_0 (citrix_workspace_unwrapped_19_6_0)
- citrix_workspace_19_8_0 (citrix_workspace_unwrapped_19_8_0)
- frogatto
- ipmiview
- ostinato
- palemoon
- python37Packages.spyder
- spyder (python38Packages.spyder)
- quartus-prime-lite
- react-native-debugger
- sage
- sageWithDoc
- softmaker-office
- sqldeveloper
- stm32cubemx
- tdm
- termius
- toggldesktop
- tome2
- ut2004demo
- worldofgoo

168 packages built:

- _90secondportraits
- airtame
- alloy (alloy5)
- alloy4
- android-studio (androidStudioPackages.stable)
- androidStudioPackages.beta
- androidStudioPackages.canary
- androidStudioPackages.dev
- anydesk
- apache-directory-studio
- assaultcube
- avocode
- basex
- beneath-a-steel-sky
- betaflight-configurator
- bitwarden
- brogue
- broken-sword-25
- ccemux
- charles (charles4)
- charles3
- clipgrab
- cura_stable
- dbeaver
- ddccontrol
- discord
- discord-canary
- discord-ptb
- dolphinEmuMaster
- dosbox
- drascula-the-vampire-strikes-back
- dreamweb
- dropbox
- dropbox-cli
- eagle
- eclipses.eclipse-committers
- eclipses.eclipse-cpp
- eclipses.eclipse-java
- eclipses.eclipse-modeling
- eclipses.eclipse-platform
- eclipses.eclipse-scala-sdk
- eclipses.eclipse-sdk
- eduke32
- evilpixie
- flight-of-the-amazon-queen
- freeoffice
- frostwire
- ganttproject-bin
- ghidra-bin
- gitkraken
- gitter
- goattracker
- goattracker-stereo
- golden-cheetah
- gomuks
- gpsprune
- groove
- hakuneko
- hyperrogue
- ivan
- jabref
- jameica
- jd-gui
- jdiskreport
- jetbrains.clion
- jetbrains.datagrip
- jetbrains.goland
- jetbrains.idea-community
- jetbrains.idea-ultimate
- jetbrains.mps
- jetbrains.phpstorm
- jetbrains.pycharm-community
- jetbrains.pycharm-professional
- jetbrains.rider
- jetbrains.ruby-mine
- jetbrains.webstorm
- jitsi
- jmol
- joplin-desktop
- keen4
- keepass
- kodestudio
- leo-editor
- lighttable
- lure-of-the-temptress
- mame
- mari0
- mate.caja-dropbox
- mgba
- michabo
- mindustry
- minecraft
- mist
- mlterm
- monero-gui
- mrrescue
- munt
- netbeans
- netlogo
- nixui
- nottetris2
- obinskit
- open-ecard
- openjk
- orthorobot
- pdfsam-basic
- pharo
- pharo-launcher
- portfolio
- postman
- prusa-slicer
- pymol
- qpaeq
- rambox-pro
- ricochet
- rimshot
- riot-desktop
- robo3t
- rocksndiamonds
- rstudio
- rstudioWrapper
- runelite
- rxvt-unicode
- rxvt-unicode-unwrapped
- saleae-logic
- scid-vs-pc
- sidequest
- sienna
- simplenote
- slic3r
- slimerjs
- smartgithg
- squirrel-sql
- ssb-patchwork
- sublime
- svxlink
- sweethome3d.application
- sweethome3d.furniture-editor
- sweethome3d.textures-editor
- swingsane
- system-syzygy
- teamspeak_client
- teleprompter
- tensor
- thunderbird
- tlaplusToolbox
- todoist-electron
- tome4
- transgui
- trilium-desktop
- tusk
- tvbrowser-bin
- vapor
- vice
- vis
- visualvm
- vscode
- vscode-with-extensions
- vscodium
- wasabiwallet
- webbrowser
- winpdb
- wire-desktop
- write_stylus
- xmind
- xournal
- zotero
- zsnes

[piegamesde]

As far as I can tell, this change does not break anything. The packages that don't build are either broken with compile errors, or broke because of #75729.

[nixos-discourse]

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review/3032/247

[worldofpeace]

Oops, I'll see if I can look at this.

[maralorn]

I like these changes a lot, let's merge them!

I have just a few nitpicks.

Besides that, let's see if there is any true dependency breakage. I feel confident enough to understand these changes to merge this, after we have reviewed the breakage.

[maralorn]

I think this is ready to merge as soon as someone can convince me, that this does not introduce any new breakage. (That'll probably be me, as soon a find the time to go through all build errors.)

[maralorn]

Wait, now that I think about it: With the true instead of "true" change we might actually be introducing type errors. We need to find them and fix them in affected packages.

[piegamesde]

Wait, now that I think about it: With the true instead of "true" change we might actually be introducing type errors. We need to find them and fix them in affected packages.

But why should it? We only only change the type of the default value, but the value itself still can be of any type.

But yes, maybe some beefy machine should run nixpkgs-review pr 91790 again to see if no packages broke in the meantime.

[maralorn]

Wait, now that I think about it: With the true instead of "true" change we might actually be introducing type errors. We need to find them and fix them in affected packages.

But why should it? We only only change the type of the default value, but the value itself still can be of any type.

It should actually break on all packages, that have specified that option before, because they have all supplied a string, but we now expect a boolean. I didn‘t actually read your code. Apparently you have thought of that. Thanks.

(Fun fact: I have learned since yesterday, that is not specified with what terminal to open a desktop file and finding a standard for that is an at least 10 year old issue in gnome with a lot of bikeshedding on xdg lists…)

[piegamesde]

It looks like that this pull request breaks palemoon, I'll look into it.

[maralorn]

thx!

[maralorn]

(For observers, we had figured out, that this does not break palemoon or any other of the above builds.)