Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

common: Add stack.yaml to common files folder and link pov as a test. #443

Merged
merged 3 commits into from
Nov 28, 2016
Merged

common: Add stack.yaml to common files folder and link pov as a test. #443

merged 3 commits into from
Nov 28, 2016

Conversation

rbasso
Copy link
Contributor

@rbasso rbasso commented Nov 28, 2016

As discussed in #442, it would be nice to have a single stack.yaml file and link all the others to it.

We expect this approach will work, but it is better to test fetching an exercise before making huge changes.

This PR does the following:

  • Add common/stack.yaml
  • Add common to ignore section in config.json
  • Fix .travis.yml to dereference links when copying files for testing
  • Symbolic link stack.yaml from exercism pov

Related to #442.

Create a folder to hold files that are common to all exercises.

- Add `common` folder to ignore section of `config.json`.
- Add `stack.yaml` to the new folder.
When copying files before testing, `cp` copies the symbolic links
instead of the target files. This adds `-L` so that relative symbolic
links will not break building the tests in Travis-CI.
@petertseng
Copy link
Member

By the way, I take it you requested for reviews to become required? I'm only guessing, since I don't think either of us would have been able to change it ourselves. No problem, just want to know when it changed

@petertseng petertseng merged commit 91a94a9 into exercism:master Nov 28, 2016
@rbasso
Copy link
Contributor Author

rbasso commented Nov 28, 2016

By the way, I take it you requested for reviews to become required? I'm only guessing, since I don't think either of us would have been able to change it ourselves. No problem, just want to know when it changed

Sorry! I was asking Katrina to add abo64 and took the opportunity to ask if it would be a good idea to enable protection for the master branch against accidentals pushes/merges.

She just enabled that, so I didn't had the time to send you a message. We can ask her to revert that if you prefer.

@rbasso rbasso deleted the common-stack-yaml-test branch November 28, 2016 18:32
@petertseng
Copy link
Member

enable protection for the master branch against accidentals pushes/merges.

These are OK! Only "Require branches to be up to date before merging" has historically slowed us down. Conversation being continued in #444

@ErikSchierboom
Copy link
Member

Interesting approach using the symlink. Hopefully, this will be a suitable solution. You put the shared file in a common directory, but doesn't that require this issue being closed?

@rbasso
Copy link
Contributor Author

rbasso commented Nov 29, 2016

My hope is that the symbolic link will magically turn in a file with the contents from the target file in the common folder. Waiting for it to go online so we can check it.

If we had the functionality describe in this comment we would be able to avoid the symbolic links completely. I'm just using the same name I used in the example there.

@ErikSchierboom
Copy link
Member

Okay, please let me know if it works, I'm really curious :)

@petertseng
Copy link
Member

I believe it will work, since I believe both of these do follow symlinks:

And I am hoping the test agrees with this.

I may want to add a test to trackler that ensures that this continues to work, if we start to depend on the functionality.

If we had the functionality describe in this comment we would be able to avoid the symbolic links completely. I'm just using the same name I used in the example there.

Will local development work without the stack.yaml present as well? For me it asks me to stack init because Error parsing targets: The specified targets matched no packages. but maybe there is a solution to this?

@rbasso
Copy link
Contributor Author

rbasso commented Nov 30, 2016

Will local development work without the stack.yaml present as well? For me it asks me to stack init because Error parsing targets: The specified targets matched no packages. but maybe there is a solution to this?

What OS / shell are you using @petertseng?
Can you please give me detailed instructions on how to reproduce the error with pov on our current master branch? Here it seems to work normally.

@petertseng
Copy link
Member

$ uname -sr
Linux charon 4.8.10-1-ARCH

Starting from the repo root:

$ cd exercises/pov
$ rm stack.yaml
$ stack test --verbose
Version 1.2.0 x86_64
Compiled with:
- Cabal-1.24.0.0
[I snipped a lot of other packages here]

2016-11-30 06:39:17.265070: [debug] Checking for project config at: /home/pt/src/xhaskell/exercises/pov/stack.yaml
@(Stack/Config.hs:792:9)
2016-11-30 06:39:17.265411: [debug] Checking for project config at: /home/pt/src/xhaskell/exercises/stack.yaml
@(Stack/Config.hs:792:9)
2016-11-30 06:39:17.265486: [debug] Checking for project config at: /home/pt/src/xhaskell/stack.yaml
@(Stack/Config.hs:792:9)
2016-11-30 06:39:17.265559: [debug] Checking for project config at: /home/pt/src/stack.yaml
@(Stack/Config.hs:792:9)
2016-11-30 06:39:17.265629: [debug] Checking for project config at: /home/pt/stack.yaml
@(Stack/Config.hs:792:9)
2016-11-30 06:39:17.265697: [debug] Checking for project config at: /home/stack.yaml
@(Stack/Config.hs:792:9)
2016-11-30 06:39:17.265764: [debug] Checking for project config at: /stack.yaml
@(Stack/Config.hs:792:9)
2016-11-30 06:39:17.265831: [debug] No project config file found, using defaults.
@(Stack/Config.hs:814:13)
2016-11-30 06:39:17.267342: [info] Run from outside a project, using implicit global project config
@(Stack/Config.hs:431:13)
2016-11-30 06:39:17.267865: [info] Using resolver: lts-7.9 from implicit global project's config file: /home/pt/.stack/global-project/stack.yaml
@(Stack/Config.hs:445:32)
2016-11-30 06:39:17.267960: [debug] Trying to decode /home/pt/.stack/build-plan-cache/x86_64-linux/lts-7.9.cache
@(Data/Store/VersionTagged.hs:68:5)
2016-11-30 06:39:17.275416: [debug] Success decoding /home/pt/.stack/build-plan-cache/x86_64-linux/lts-7.9.cache
@(Data/Store/VersionTagged.hs:72:13)
2016-11-30 06:39:17.279329: [debug] Getting system compiler version
@(Stack/Setup.hs:354:17)
2016-11-30 06:39:17.279537: [debug] Run process: /usr/bin/ghc --info
@(System/Process/Read.hs:277:3)
2016-11-30 06:39:17.341295: [debug] Process finished in 61 ms: /usr/bin/ghc --info
@(System/Process/Read.hs:277:3)
2016-11-30 06:39:17.346666: [debug] Asking GHC for its version
@(Stack/Setup/Installed.hs:101:13)
2016-11-30 06:39:17.346894: [debug] Getting Cabal package version
@(Stack/GhcPkg.hs:171:5)
2016-11-30 06:39:17.346984: [debug] Run process: /usr/bin/ghc --numeric-version
@(System/Process/Read.hs:277:3)
2016-11-30 06:39:17.347339: [debug] Getting global package database location
@(Stack/GhcPkg.hs:54:5)
2016-11-30 06:39:17.348023: [debug] Run process: /usr/bin/ghc-pkg --no-user-package-db field --simple-output Cabal version
@(System/Process/Read.hs:277:3)
2016-11-30 06:39:17.348485: [debug] Run process: /usr/bin/ghc-pkg --no-user-package-db list --global
@(System/Process/Read.hs:277:3)
2016-11-30 06:39:17.374714: [debug] Process finished in 26 ms: /usr/bin/ghc-pkg --no-user-package-db field --simple-output Cabal version
@(System/Process/Read.hs:277:3)
2016-11-30 06:39:17.377416: [debug] Process finished in 28 ms: /usr/bin/ghc-pkg --no-user-package-db list --global
@(System/Process/Read.hs:277:3)
2016-11-30 06:39:17.390906: [debug] Process finished in 43 ms: /usr/bin/ghc --numeric-version
@(System/Process/Read.hs:277:3)
2016-11-30 06:39:17.391374: [debug] Resolving package entries
@(Stack/Setup.hs:234:5)
2016-11-30 06:39:17.391997: [debug] Starting to execute command inside EnvConfig
@(Stack/Runners.hs:166:18)
2016-11-30 06:39:17.392067: [debug] Parsing the cabal files of the local packages
@(Stack/Build/Source.hs:282:5)
2016-11-30 06:39:17.392115: [debug] Parsing the targets
@(Stack/Build/Source.hs:219:5)
Error parsing targets: The specified targets matched no packages.
Perhaps you need to run 'stack init'?

Well, sure I could stack init, though that will write a new stack.yaml, not sure that's what we want.

I could try to have a stack.yaml higher up:

$ cp ../allergies/stack.yaml ..
$ stack test
No .cabal file found in directory /home/pt/src/xhaskell/exercises/

Okay, guess not.

By the way it acts as expected if I leave stack.yaml as-is (the symlink).

@petertseng
Copy link
Member

petertseng commented Nov 30, 2016

We can see whether this change is live:

exercism/DEPRECATED.trackler@0f08803#diff-c6c56fb57dc791175a5882b2f7b448d7

As you can see, the change to symlink pov/stack.yaml will come with my updates to bob/test/Tests.hs and sublist/test/Tests.hs, so we can see whether the Bob test has been updated, and if it is, then proceed to test POV.

The latest changes are live (Sublist and Bob have 2016-11-29 markers)

http://x.exercism.io/v2/exercises/haskell/pov doesn't have stack.yaml, which is surprising.\

It's something to do with the way the gem is packaged. I do gem install trackler, then look at ~/.gem/ruby/2.3.0/gems/trackler-2.0.3.0/tracks/haskell/exercises/pov, and stack.yaml points to tracks/haskell/exercises/pov/../../common/stack.yaml there, which is unfortunately incorrect.

@rbasso
Copy link
Contributor Author

rbasso commented Dec 3, 2016

The current situation is that pov has a missing stack.yaml file when fetched. There in a small chance (~1/30 per day) that a user will try to solve it.

Considering that our experience with symbolic links failed, show we revert this PR or do we have any expectation that this will be fixed in a short time?

@petertseng
Copy link
Member

Since the current obstacle is that RubyGems needs to release a fixed version (the current one 2.6.8 apparently has the incorrect behaviour), it's not certain this can be fixed in a short time.

Alternatively we could ask @kytrinyx to use a different version of RubyGems to package the trackler gem, one >= 2.5.2 and <= 2.6.4, if willing, but it could be an undue burden.

@rbasso
Copy link
Contributor Author

rbasso commented Dec 4, 2016

Considering that we still have the hope that this will be fixed, instead of reverting everything, I'm considering just restoring pov's stack.yaml and add a dummy symbolic link to common/stack.yaml. This way we'll be able to see when the problem is solved.

petertseng pushed a commit that referenced this pull request Dec 4, 2016
The experiment proposed in #442 of symbolic linking the
`stack.yaml` file has failed, as the file was not fetched
as expected.

This PR partially reverts #443, but leaves the fix
in `.travis.yml` and the `common/stack.yaml` file.
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