Icons in /usr/share/icons/hicolor/256x256/ don't seem to work

[Krzmbrzl]

If I read the code correctly, then the code looks for the presence of the icon specified in the desktop file in appdir + /usr/share/icons/hicolor/256x256/ here:

go-appimage/src/appimagetool/appimagetool.go

Lines 369 to 382 in f1ebcce

	} else if helpers.CheckIfFileExists(appdir + "/usr/share/icons/hicolor/256x256/" + iconname + ".png") {
	// Search in usr/share/icons/hicolor/256x256 and copy from there
	input, err := ioutil.ReadFile(appdir + "/usr/share/icons/hicolor/256x256/" + iconname + ".png")
	if err != nil {
	log.Println(err)
	return
	}
	err = ioutil.WriteFile(appdir+".DirIcon", input, 0644)
	if err != nil {
	log.Println("Error copying ticon to", appdir+".DirIcon")
	log.Println(err)
	return
	}
	} else {

What strikes me though is that in the first check it is checked whether the icon exists at the root of the appdir. This in and of itself is not what I find interesting. Rather the fact that this code part sets the iconFile variable:

go-appimage/src/appimagetool/appimagetool.go

Lines 367 to 369 in f1ebcce

	if helpers.CheckIfFileExists(appdir+"/"+iconname+".png") == true {
	iconfile = appdir + "/" + iconname + ".png"
	} else if helpers.CheckIfFileExists(appdir + "/usr/share/icons/hicolor/256x256/" + iconname + ".png") {

The second check however doesn't seem to set this variable. Thus

go-appimage/src/appimagetool/appimagetool.go

Line 386 in f1ebcce

log.Println("Icon file:", iconfile)

Prints

Icon file: 

aka: An empty String.

A few lines down however this variable is used to assemble a path to copy:

go-appimage/src/appimagetool/appimagetool.go

Line 397 in f1ebcce

err = helpers.CopyFile(iconfile, appdir+"/.DirIcon")

In my case this results in this error:

ERROR Copy .DirIcon: read .: is a directory

I also think it is interesting that the code above seems to create a .DirIcon file in the AppImage's root

go-appimage/src/appimagetool/appimagetool.go

Lines 371 to 381 in f1ebcce

	input, err := ioutil.ReadFile(appdir + "/usr/share/icons/hicolor/256x256/" + iconname + ".png")
	if err != nil {
	log.Println(err)
	return
	}
	err = ioutil.WriteFile(appdir+".DirIcon", input, 0644)
	if err != nil {
	log.Println("Error copying ticon to", appdir+".DirIcon")
	log.Println(err)
	return
	}

and a bit further down we have

go-appimage/src/appimagetool/appimagetool.go

Lines 391 to 394 in f1ebcce

	if helpers.CheckIfFileExists(appdir+"/.DirIcon") == true {
	log.Println("Deleting pre-existing .DirIcon")
	os.Remove(appdir + "/.DirIcon")
	}

which seems to exactly cancel above action. Interestingly however I did not see the Deleting pre-existing .DirIcon message in my logs so maybe I am missing something here 🤔

All in all I think that maybe the functionality for icons in the respective subdir is broken atm...

[Krzmbrzl]

My suggestion to fix this would simply to set the fileName variable in the second choice as is done in the first one. However I don't understand the .DirIcon logic so I am hesitant to patch this in and provide a PR...

[probonopd]

Each AppDir, including the one inside an AppImage, needs to have a .DirIcon file. Hence appimagetool tries to find a suitable icon and copy it there.

I will have a look at the logic flaws you are pointing out later.

Reference:
https://docs.appimage.org/reference/appdir.html#general-description

[probonopd]

Say you have icon=mygreatapp in your .desktop file. Then the corresponding icon should be in <AppDir>/usr/share/icons/hicolor/256x256/apps/mygreatapp.png because that is where desktop integration mechanisms like appimaged will look for it.

[Krzmbrzl]

In that case I think there's another bug in the code as the check at

go-appimage/src/appimagetool/appimagetool.go

Line 369 in f1ebcce

} else if helpers.CheckIfFileExists(appdir + "/usr/share/icons/hicolor/256x256/" + iconname + ".png") {

doesn't check in .../256x256/apps but only in .../256x256 ☝️

[probonopd]

Good catch 👍