Re: Review Request 128112: New module: ecm_win_resolve_symlinks

[Gleb Popov]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128112/
---

(Updated Nov. 5, 2016, 2:40 p.m.)


Status
--

This change has been marked as submitted.


Review request for Extra Cmake Modules and KDE Frameworks.


Changes
---

Submitted with commit 632b8868de38e2f85fb4969bcae3dd89a2a0695c by Gleb Popov to 
branch master.


Repository: extra-cmake-modules


Description
---

When git is checking out repositories with UNIX symbolic links inside on 
Windows machine, it writes them as plain text files, containing relative path 
to the real file. This is the case for breeze-icons framework, for instance, 
and this breaks some icons that are symlinked.

This macro is intended to fix that. There is some room for performance 
improvement, but i wanted to get the feedback early.


Diffs
-

  modules/ECMWinResolveSymlinks.cmake PRE-CREATION 

Diff: https://git.reviewboard.kde.org/r/128112/diff/


Testing
---


Thanks,

Gleb Popov

Re: Review Request 128112: New module: ecm_win_resolve_symlinks

[David Faure]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128112/#review100590
---


Fix it, then Ship it!





modules/ECMWinResolveSymlinks.cmake (line 21)


needs to be updated


- David Faure


On June 17, 2016, 8:32 a.m., Gleb Popov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128112/
> ---
> 
> (Updated June 17, 2016, 8:32 a.m.)
> 
> 
> Review request for Extra Cmake Modules and KDE Frameworks.
> 
> 
> Repository: extra-cmake-modules
> 
> 
> Description
> ---
> 
> When git is checking out repositories with UNIX symbolic links inside on 
> Windows machine, it writes them as plain text files, containing relative path 
> to the real file. This is the case for breeze-icons framework, for instance, 
> and this breaks some icons that are symlinked.
> 
> This macro is intended to fix that. There is some room for performance 
> improvement, but i wanted to get the feedback early.
> 
> 
> Diffs
> -
> 
>   modules/ECMWinResolveSymlinks.cmake PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/128112/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Gleb Popov
> 
>

Re: Review Request 128112: New module: ecm_win_resolve_symlinks

[Gleb Popov]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128112/#review99356
---



Bump.

- Gleb Popov


On June 17, 2016, 11:32 a.m., Gleb Popov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128112/
> ---
> 
> (Updated June 17, 2016, 11:32 a.m.)
> 
> 
> Review request for Extra Cmake Modules and KDE Frameworks.
> 
> 
> Repository: extra-cmake-modules
> 
> 
> Description
> ---
> 
> When git is checking out repositories with UNIX symbolic links inside on 
> Windows machine, it writes them as plain text files, containing relative path 
> to the real file. This is the case for breeze-icons framework, for instance, 
> and this breaks some icons that are symlinked.
> 
> This macro is intended to fix that. There is some room for performance 
> improvement, but i wanted to get the feedback early.
> 
> 
> Diffs
> -
> 
>   modules/ECMWinResolveSymlinks.cmake PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/128112/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Gleb Popov
> 
>

Re: Review Request 128112: New module: ecm_win_resolve_symlinks

[Gleb Popov]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128112/
---

(Updated June 17, 2016, 11:32 a.m.)


Review request for Extra Cmake Modules.


Changes
---

Address comments.


Repository: extra-cmake-modules


Description
---

When git is checking out repositories with UNIX symbolic links inside on 
Windows machine, it writes them as plain text files, containing relative path 
to the real file. This is the case for breeze-icons framework, for instance, 
and this breaks some icons that are symlinked.

This macro is intended to fix that. There is some room for performance 
improvement, but i wanted to get the feedback early.


Diffs (updated)
-

  modules/ECMWinResolveSymlinks.cmake PRE-CREATION 

Diff: https://git.reviewboard.kde.org/r/128112/diff/


Testing
---


Thanks,

Gleb Popov

___
Kde-buildsystem mailing list
Kde-buildsystem@kde.org
https://mail.kde.org/mailman/listinfo/kde-buildsystem

Re: Review Request 128112: New module: ecm_win_resolve_symlinks

[Hannah von Reth]

> On June 8, 2016, 5:26 a.m., Nicolás Alvarez wrote:
> > I looked at this a few times, researched a bit about git + symlinks + 
> > Windows... and I conclude that _having symlinks in the repository_ is a bad 
> > idea, because of the problems they cause on Windows, and this RR is not a 
> > good enough workaround for those problems.
> > 
> > Instead, I propose:
> > 1. delete the symlinks from the `breeze-icons` git repository
> > 2. create a text file with some sort of list of "icon aliases"
> > 3. make the build system create the symlinks at compile time (or install 
> > time) based on the icon alias list, or copies (or NTFS hardlinks?) in the 
> > case of Windows
> > 4. add server-side git hooks to prevent symlinks from being introduced again
> > 
> > ...which would be a big enough change that it needs discussion in mailing 
> > lists, and not just here :)
> 
> Gleb Popov wrote:
> The module does basically exactly this, just builds up a list of symbolic 
> links automatically. Why delete them, then?

I did the similar thing on the installed files but this approach is much better.
I discussed that issue at randa 2015 with the breeze people and I agree that 
this is the best work flow for them.

I think this script is what we need but I'd say we need feedback from the ecm 
team.


- Hannah


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128112/#review96294
---


On June 8, 2016, 5:08 a.m., Gleb Popov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128112/
> ---
> 
> (Updated June 8, 2016, 5:08 a.m.)
> 
> 
> Review request for Extra Cmake Modules.
> 
> 
> Repository: extra-cmake-modules
> 
> 
> Description
> ---
> 
> When git is checking out repositories with UNIX symbolic links inside on 
> Windows machine, it writes them as plain text files, containing relative path 
> to the real file. This is the case for breeze-icons framework, for instance, 
> and this breaks some icons that are symlinked.
> 
> This macro is intended to fix that. There is some room for performance 
> improvement, but i wanted to get the feedback early.
> 
> 
> Diffs
> -
> 
>   modules/ECMWinResolveSymlinks.cmake PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/128112/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Gleb Popov
> 
>

___
Kde-buildsystem mailing list
Kde-buildsystem@kde.org
https://mail.kde.org/mailman/listinfo/kde-buildsystem

Re: Review Request 128112: New module: ecm_win_resolve_symlinks

[Hannah von Reth]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128112/#review96460
---




modules/ECMWinResolveSymlinks.cmake (line 69)


whitespace


- Hannah von Reth


On June 8, 2016, 5:08 a.m., Gleb Popov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128112/
> ---
> 
> (Updated June 8, 2016, 5:08 a.m.)
> 
> 
> Review request for Extra Cmake Modules.
> 
> 
> Repository: extra-cmake-modules
> 
> 
> Description
> ---
> 
> When git is checking out repositories with UNIX symbolic links inside on 
> Windows machine, it writes them as plain text files, containing relative path 
> to the real file. This is the case for breeze-icons framework, for instance, 
> and this breaks some icons that are symlinked.
> 
> This macro is intended to fix that. There is some room for performance 
> improvement, but i wanted to get the feedback early.
> 
> 
> Diffs
> -
> 
>   modules/ECMWinResolveSymlinks.cmake PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/128112/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Gleb Popov
> 
>

___
Kde-buildsystem mailing list
Kde-buildsystem@kde.org
https://mail.kde.org/mailman/listinfo/kde-buildsystem

Re: Review Request 128112: New module: ecm_win_resolve_symlinks

[Gleb Popov]

> On June 8, 2016, 8:26 a.m., Nicolás Alvarez wrote:
> > I looked at this a few times, researched a bit about git + symlinks + 
> > Windows... and I conclude that _having symlinks in the repository_ is a bad 
> > idea, because of the problems they cause on Windows, and this RR is not a 
> > good enough workaround for those problems.
> > 
> > Instead, I propose:
> > 1. delete the symlinks from the `breeze-icons` git repository
> > 2. create a text file with some sort of list of "icon aliases"
> > 3. make the build system create the symlinks at compile time (or install 
> > time) based on the icon alias list, or copies (or NTFS hardlinks?) in the 
> > case of Windows
> > 4. add server-side git hooks to prevent symlinks from being introduced again
> > 
> > ...which would be a big enough change that it needs discussion in mailing 
> > lists, and not just here :)

The module does basically exactly this, just builds up a list of symbolic links 
automatically. Why delete them, then?


- Gleb


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128112/#review96294
---


On June 8, 2016, 8:08 a.m., Gleb Popov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128112/
> ---
> 
> (Updated June 8, 2016, 8:08 a.m.)
> 
> 
> Review request for Extra Cmake Modules.
> 
> 
> Repository: extra-cmake-modules
> 
> 
> Description
> ---
> 
> When git is checking out repositories with UNIX symbolic links inside on 
> Windows machine, it writes them as plain text files, containing relative path 
> to the real file. This is the case for breeze-icons framework, for instance, 
> and this breaks some icons that are symlinked.
> 
> This macro is intended to fix that. There is some room for performance 
> improvement, but i wanted to get the feedback early.
> 
> 
> Diffs
> -
> 
>   modules/ECMWinResolveSymlinks.cmake PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/128112/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Gleb Popov
> 
>

___
Kde-buildsystem mailing list
Kde-buildsystem@kde.org
https://mail.kde.org/mailman/listinfo/kde-buildsystem

Re: Review Request 128112: New module: ecm_win_resolve_symlinks

[Nicolás Alvarez]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128112/#review96294
---



I looked at this a few times, researched a bit about git + symlinks + 
Windows... and I conclude that _having symlinks in the repository_ is a bad 
idea, because of the problems they cause on Windows, and this RR is not a good 
enough workaround for those problems.

Instead, I propose:
1. delete the symlinks from the `breeze-icons` git repository
2. create a text file with some sort of list of "icon aliases"
3. make the build system create the symlinks at compile time (or install time) 
based on the icon alias list, or copies (or NTFS hardlinks?) in the case of 
Windows
4. add server-side git hooks to prevent symlinks from being introduced again

...which would be a big enough change that it needs discussion in mailing 
lists, and not just here :)

- Nicolás Alvarez


On Junio 8, 2016, 2:08 a.m., Gleb Popov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128112/
> ---
> 
> (Updated Junio 8, 2016, 2:08 a.m.)
> 
> 
> Review request for Extra Cmake Modules.
> 
> 
> Repository: extra-cmake-modules
> 
> 
> Description
> ---
> 
> When git is checking out repositories with UNIX symbolic links inside on 
> Windows machine, it writes them as plain text files, containing relative path 
> to the real file. This is the case for breeze-icons framework, for instance, 
> and this breaks some icons that are symlinked.
> 
> This macro is intended to fix that. There is some room for performance 
> improvement, but i wanted to get the feedback early.
> 
> 
> Diffs
> -
> 
>   modules/ECMWinResolveSymlinks.cmake PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/128112/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Gleb Popov
> 
>

___
Kde-buildsystem mailing list
Kde-buildsystem@kde.org
https://mail.kde.org/mailman/listinfo/kde-buildsystem

Re: Review Request 128112: New module: ecm_win_resolve_symlinks

[Gleb Popov]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128112/
---

(Updated June 8, 2016, 8:08 a.m.)


Review request for Extra Cmake Modules.


Changes
---

Some performance optimizations and bug fixes.


Repository: extra-cmake-modules


Description
---

When git is checking out repositories with UNIX symbolic links inside on 
Windows machine, it writes them as plain text files, containing relative path 
to the real file. This is the case for breeze-icons framework, for instance, 
and this breaks some icons that are symlinked.

This macro is intended to fix that. There is some room for performance 
improvement, but i wanted to get the feedback early.


Diffs (updated)
-

  modules/ECMWinResolveSymlinks.cmake PRE-CREATION 

Diff: https://git.reviewboard.kde.org/r/128112/diff/


Testing
---


Thanks,

Gleb Popov

___
Kde-buildsystem mailing list
Kde-buildsystem@kde.org
https://mail.kde.org/mailman/listinfo/kde-buildsystem