exercism / cli

A Go based command line tool for exercism.org.
https://exercism.org/docs/using/solving-exercises/working-locally
MIT License
1.34k stars 361 forks source link

Setup chmod for certain files in downloaded exercise. #903

Open dector opened 4 years ago

dector commented 4 years ago

According to sources, exercism tool is just downloading files. But sometimes it's really nice to make certain files executable (see exercism/4991).

kytrinyx commented 4 years ago

This has been discussed previously, and the conclusion at the time was to create files as non-executable: https://github.com/exercism/cli/issues/276

uzilan commented 4 years ago

I can't say I understand why, as running the gradlew scripts should be the way mentees run the tests, isn't it? Why make it harder?

In any case, I think we need to include instructions which explain how to chmod the files if we decide to make them non-executable.

dector commented 4 years ago

I agree with @uzilan here. There are some traideoffs for running executable scripts fetched from remote source, indeed. But I think people will not check executable files anyway.

sshine commented 4 years ago

Summary and evaluation of argument made in #276

276 was (presumably) created because some tracks commit some files as executable because track maintainers assume file permissions are retained upon download. But they're not. To resolve this discrepancy we should either retain file permissions upon download, or at least have a principle to not do so.

Making script files executable via a #! shebang is one of convenience: We want students to type less to run tests. This is (presumably) the motivation for some track maintainers to commit certain files as executable, and for the present issue to be raised. If there are other reasons they should be mentioned.

The only argument against this, mentioned in #276, is one of perceived security:

I don't like to encourage people to simply run an executable they've fetched from a remote.

... which they would be doing anyway, even when prefixed with an interpreter. It'd be code from a remote location executed via an interpreter, whether through ./file or interp file. So while there's nothing more or less secure about it, executing a script that is downloaded is perceived to be more dangerous if run directly.

I think the argument against executable scripts can be reduced to an aesthetical preference.

Windows compatibility and consistency of documentation

I present another argument against making scripts directly executable.

While ./file is an option on Linux/MacOSX/BSD, it isn't in Windows unless you set PATHEXT=... and the executable has a file ending that is listed in that environment variable.

This won't be a problem as long as Windows users can execute tests whichever way is most convenient on Windows, and Linux/MacOSX/BSD users execute tests in the same way or with ./file. But as most tracks have a section in config/exercise_readme.go.tmpl that explains how to run the tests, this gives track maintainers at least the following options:

  1. Don't mention the possibility to run tests with ./file and let people figure this out themselves, e.g. by noticing that executables are highlighted with a different color in their terminal, if that is the case.
  2. Mention both options, but say that the ./file option only applies to some operating systems.
  3. Only mention ./file as an option, which would be shorter and more centric to operating systems that support it, and less supportive of Windows where we probably want to be more supportive.
  4. Something else entirely.

Since we don't know the operating system of the student as we present the README (on the website and in the copy downloaded by the CLI), generating, maintaining and presenting alternative hints for different operating systems seems like a lot of work when a single instruction does exist for multiple operating systems.

For which tracks does this apply?

This discussion applies to tracks for which the test suite is directly executable because the programming language (or choice of framework around it) does not provide a test command. For example, in Haskell the chosen command is stack test, in Perl 5 it's prove ., in Java it's gradle test, and so on.

Having executable test files would mainly benefit tracks where testing equipment is not provided in a separate command. For that reason it would be nice to know which track stands to benefit here?

Performing this change should, in my opinion, be based on the observation that it would benefit a track that does not have a better way to execute tests. This would most likely be an archaic scripting language. (Archaic: Has no modern test framework. Scripting: Allows #! shebangs in source code files.)

Gradle on Kotlin track

@uzilan mentions gradlew (aka Gradle wrapper), which I haven't tried myself, but it appears to be a build system that supports many languages. Doing a global find I can only see that Java and Kotlin tracks use Gradle. Java track's Installing Java advices to run choco install gradle and subsequently use gradle test rather than gradlew / gradlew.bat, which was previously distributed along with the exercises as it is in Kotlin track now.

So in the case of Kotlin track (which I presume @uzilan to represent in this issue), how about doing the same for Kotlin track as Java track does? I see that Kotlin track's Installing Kotlin does not mention choco install gradle but does recommend choco install openjdk11.

This is not to disqualify the proposal, but in the particular case of Kotlin track, isn't there actually a better choice?

("Why make it harder?" is a biased question because it assumes that students, on average, see that files are exetuable and know to run them with ./file, rather than simply do whatever the problem description suggests. Having mentored many students in the real world, it always surprises me how little people know about their file systems.)

dector commented 4 years ago

Thanks @sshine for such extended answer.

In Kotlin/Java world Gradle is de-facto a default build system. (Probably some legacy enterprise projects still use something like Maven).

Using ./gradlew/gradlew.bat is the recommended way to run Gradle:

The recommended way to execute any Gradle build is with the help of the Gradle Wrapper (in short just “Wrapper”). The Wrapper is a script that invokes a declared version of Gradle, downloading it beforehand if necessary.

In a nutshell you gain the following benefits:

  • Standardizes a project on a given Gradle version, leading to more reliable and robust builds.

  • Provisioning a new Gradle version to different users and execution environment (e.g. IDEs or Continuous Integration servers) is as simple as changing the Wrapper definition.

With wrapper we can configure any required Gradle version to use with our build-script. It allows us to be sure that our build scenarios works as expected even if student have outdated/another Gradle version installed. Invoking Gradle is a workaround I'd rather want to avoid. It's better to include instructions how to make file executable, I guess.

uzilan commented 4 years ago

It is also important to maintain a certain version when we build the java and kotlin test runners. Otherwise we would have to guess which version of gradle the mentees are using to build and test their solutions. Using gradlew we can assert a certain gradle version.

sanderploegsma commented 6 months ago

Just wanted to weigh in here saying that the Java track is now also using the Gradle wrapper and no longer instructs students to install Gradle locally. This means we also run into the issue where the Gradle wrapper script has to be marked executable by the student for each exercise they download.