Skip to content

Create icons folder if it doesn't exist#367

Merged
Et0h merged 1 commit intoSyncplay:masterfrom
Lctrs:patch-1
Nov 29, 2020
Merged

Create icons folder if it doesn't exist#367
Et0h merged 1 commit intoSyncplay:masterfrom
Lctrs:patch-1

Conversation

@Lctrs
Copy link
Contributor

@Lctrs Lctrs commented Nov 25, 2020

If it doesn't exist first, hicolor will be stripped from the path by the cp command

If it doesn't exist first, `hicolor` will be stripped from the path by the `cp` command
@Et0h Et0h requested a review from daniel-123 November 25, 2020 13:19
@Et0h Et0h assigned Et0h and daniel-123 and unassigned Et0h Nov 25, 2020
@Et0h
Copy link
Contributor

Et0h commented Nov 25, 2020

I'm expecting this is a non-controversial change, but as I use Windows it would be useful if someone with makefile experience can confirm that this is the correct approach.

@Et0h
Copy link
Contributor

Et0h commented Nov 25, 2020

@Lctrs What OS are you using which you believe exhibits this behaviour (of cp not creating the hicolor folder when the icon folder does not exist) and have you confirmed it?

@Lctrs
Copy link
Contributor Author

Lctrs commented Nov 26, 2020

When trying to create a flatpak for the client. The icons folder doesn't exist in this case beforehand, and the cp command rename the hicolor folder to icons, resulting in an invalid tree.

Copy link
Contributor

@daniel-123 daniel-123 left a comment

Choose a reason for hiding this comment

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

I've not seen this crop up up until now mostly because the assumption behind this installer script was that it will typically run on a "normal" operating system. Which pretty much always already has some software with icons, so I didn't think about situation where it doesn't exist.

It think that overall this is completely reasonable change. Though we don't have an official flatpak build process, so there is some risk of your package breaking unintentionally with some other changes in future.

@Et0h Et0h merged commit 0d4eb67 into Syncplay:master Nov 29, 2020
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.

3 participants