On Fri, Jun 19, 2020 at 2:30 AM Junio C Hamano <gitster@xxxxxxxxx> wrote:
>
> Sibi Siddharthan <sibisiddharthan.github@xxxxxxxxx> writes:
>
> > The reason I deferred modifying in PATCH v4 was because there was no
> > easy way(cross platform)
> > to change file permissions. The workaround is to juggle the files to a
> > temporary directory, and then copy them
> > to where they are intended to be with the required permissions. This
> > added quite a bit of code.
> > Since Windows platform was the priority, I did not address this issue.
>
> I recall asking you, in a very early iteration, if you just
> concentrate on Windows and do not even pretend you support Linux or
> any other systems, the series can be kept smaller and review
> simpler, and your answer was "no, it can be done with just a little
> extra code". But I think you are saying it needs "workaround" and
> what needs to happen in the workaround sounds quite ugly to me.
>
I know what I said. This is the worst edge case of this scenario.
> Let's not worry about cross-platform and instead stick to Windows
> and nothing else for now to expedite the process. As long as it is
> advertised as such, nobody would complain that it does not work on
> Linux or macOS.
>
Okay.
Just for your information.
This is the exact diff that needs to be added to address the issue.
------------------------------------------
@@ -761,6 +761,14 @@
file(STRINGS ${CMAKE_SOURCE_DIR}/git-p4.py content NEWLINE_CONSUME)
string(REPLACE "#!/usr/bin/env python" "#!/usr/bin/python" content
"${content}")
file(WRITE ${CMAKE_BINARY_DIR}/git-p4 ${content})
+#Give the scripts execute permissions. This does not apply on Windows
+if(UNIX)
+ foreach(script ${git_sh_scripts} git-instaweb
${git_perl_scripts} git-p4)
+ add_custom_target(${script}_executable ALL
+ chmod +x ${CMAKE_BINARY_DIR}/${script})
+ endforeach()
+endif()
+
#perl modules
file(GLOB_RECURSE perl_modules "${CMAKE_SOURCE_DIR}/perl/*.pm")
@@ -790,6 +799,12 @@
foreach(tm ${templates})
configure_file(${CMAKE_SOURCE_DIR}/templates/${tm}
${CMAKE_BINARY_DIR}/templates/blt/${blt_tm} @ONLY)
endforeach()
+#Give the templates read & execute permissions. This does not apply on Windows
+if(UNIX)
+ add_custom_target(templates_executable ALL
+ chmod -R +rx ${CMAKE_BINARY_DIR}/templates/blt/)
+endif()
+
#translations
if(MSGFMT_EXE)
---------------------------------------------------------
If this is simple enough, I will add it in the next patch series. If
not, when in the future should I implement this?
Thank You,
Sibi Siddharthan
