-
Notifications
You must be signed in to change notification settings - Fork 16
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
Work in Progess: Synchronize Haskell version to new changes #5 #19
base: master
Are you sure you want to change the base?
Conversation
2faf2ad
to
29791b3
Compare
I look forward to all passing build. Will have to review using my rudimentary knowledge of Haskell. Question: I noticed we have several lts resolver's named for testing in Travis. I also noted a PR in the main that has suggested bump the stack to 10.8 (latest appears to be 11.12). I assume an LTS number is a snapshot of Haskell version + packages. Should we be testing more major cuts closer to nightly? Also currently stack is only 5.0, should that bump too? I don't know how close users follow/lag. Do they come with performance improvements usually or just new features? |
Your guess is right, so if i update the resolver i need to check if it still compiles and if not, i need to see what i have to change in order to use the newer lib versions. So there could be a bit of a performance improvement. Maybe we can also turn on some aggressive optimization flags, with the drawback of a increased binary size. I'll check that out |
Okay with the updated compiler flags and upgraded ghc version i am able to achieve run times as low as "cpu 0.006 total". The best run without these opts was "cpu 0.011 total". Please note that i disabled threading, the program now only uses a single core. This is (counter-intuitively) faster than multi-threaded execution, because of the very short runtime of the program and the overhead that is introduced by starting a new cpu thread. |
Excellent, faster always better. Thanks again for working on the patch. Once merged I'll update our main fork issue and title. Sure to be welcome news to the Haskell users and those with slower CPUs. |
I think, since haskell is a compiled language which allows for multiple compiler versions via stack, we can skip the matrix tests for the haskell build and do only one build with the resolver that we specify (e.g. 11.12). That is, because everyone always needs to recompile the source code on feature updates and we specify the exact set of library versions and compiler via resolver. It does not matter (stack 🥇) which version of ghc (and haskell libs) are globally installed via the distros package maneger, therefore no need for backward or upward compatibility. I think matrix builds with different versions only make sense if you have one global interpreter or compiler installed like (python 3.5 or node 8) and you execute the program code with that version. In this case it makes ultimately sense to support many versions since users have many different interpreter versions installed. |
Good point regarding matrix. Quite handy having everything specified and isolated to within the project. Thankfully for python almost any recent python (except 3.1) should work. I had a quick look at your PR and I ran into the following issue when I tried to run
I was using the packaged stack with Linux Mint 18.3, version output:
I downloaded locally the latest stack archive, and it ran fine. That stack's version is:
I assume there is some min version of stack required to build based on current configuration, that is 1.7.1 > min > 0.1.10 . README only points to local docs on installing stack. Considering this simple event, I'm wondering if we shouldn't have a simple generic shell script like "build_haskell.sh" that does the curl/unarchive to a folder like At end of script we can emit a reminder of the line required to toggle the prompt to using Haskell. Just an idea for now, I'll continue looking at your master with 1.7.1 for now. |
Yup, there is also an issue for this error: Yes, we could create a build shell script that fetches a recent version of stack, but i would find that a bit offensive to install another recent stack version to the users host system that might somehow interfere with stack that is installed via the package manager. I suggest one of the following options:
Example: FROM haskell:8.4.3
RUN mkdir -p /app
COPY . /app
WORKDIR /app
RUN stack setup && stack build |
I took this for a spin and found a few small issues and my comments follow. Overall I'm really happy, thanks again for doing this. To test the branch I used my Test Env: Stack 1.7.1, Git 2.7.1 Bug Enable ZSH with GIT_PROMPT_EXECUTABLE=haskell, cd to a non git folder and haskell prompt crashes with error. Should silently emit no text. I'm not sure if this issue predates this branch, saw similar issue on Olivier a while ago. See image below. Bug Rebase indicator, numbers seem reversed. Should be string '1/2' (in my test case), returns '2/1'. Simple order error, reverse them. Bug Git repo has 1 incoming change from remote. The Haskell prompt reports 1 outgoing instead. This appears to be a simple order transposition again. Both numbers affected. To be clear, that means in the text output position 2 is number behind the remote, and position 3 is number ahead. Additional Comments
On the topic of the more recent stack/build script, docker idea is interesting. I will look at that, I remain not super versed with docker. In general, I prefer any change that reduces user error chance to zero (even if they are programmers). That would however make us dependent then on docker, which may or may not be installed. Alternatively, the Haskell project maintains simple fetch script (https://get.haskellstack.org/) it takes a destination flag optionally. It verifies dependencies and downloads right binary. We can use that in the script to fetch latest stack to |
Alright, i refer to your mentioned bug as bug 0,1,2 in the order you wrote them. Bug 0 yup, somehow i thought during programming that being not in a git repository is considered to be an error, but clearly that is not the case so i corrected this. Now it is simply exit 0 without any message. Bug 1 fixed that Bug 2 hmm, the commits ahead and behind logic is from olivierverdier, is it a bug that is already in the current master? I can fix that no problem, but i just want to go sure. Additional Comments Okay i'll try to write some build script like that, if this approach works without any issues and if its only drawback is a little bandwidth usage, then it would be okay for me. |
@madnight My bad on bug 2, apparently way back during my changes I pulled an order change on the Regarding build script and lts resolver bump, I think we can take this to a separate issue. For this PR probably easiest to just stay on 5 for now and resolve it later. I'll copy over ideas and we can discuss/fix separately. This one already covering a lot of changes, rather it stayed focused. |
Okay I downgraded by dropping the upgrade commit. |
I think bats https://github.com/sstephenson/bats is a nice minimal general purpose testing framework for cli applications. The good thing is that we write the test once and execute them against python and the haskell version, so no need for different tests for each language. That also eliminates the possibility of errors due to differences in the test sets between py and hs. Example: @test "non git dir" {
rm -rf .git
run gitstatus
[ "$status" -eq 0 ]
[ "$(gitstatus | head -c1 | wc -c)" -eq 0 ] # empty output
}
@test "1 commit" {
add_commit "first"
[ "$(gitstatus)" == "master 0 0 0 0 0 0 0 1 .. 0 0" ]
} I added a squash commit with some tests, i'll write more soon. |
That seems like a great idea for testing. The only downside is that this kind of test framework won't integrate with the langauge and give us a line hit/miss count like currently I can get with py.test coverage. It is however nice to have a unified functional test suite that can guarantee the interface is working as expected on both though. Will also reduce some duplication and we can extract the functional tests from the unit frameworks. I say go for it, I can always add it as a fourth environment to the tox runner and we'll just have a bit of overlap. Feel free to make liberal use of my |
By the way what do you think about a commit prefix like "Hask:" or something like that, so that we can differentiate between python and haskell commits in the git history? |
@madnight That's fine by me if you'd like that. I don't think it a large deal, will be pretty clear when Haskell or other being worked on. How goes the progress on bats tests? It been a bit of a while. I been a bit busy myself last bit. I suppose I can check out your branch and try rewriting my functional tests inside bats and PR you. |
Yup, busy too here. Okay i rebased with your master again and added the prefix, i think this way it will be easier to see which commits are related to haskell in a mixed history, otherwise messages like Yeah, it would be great if you could write some tests in bats : ) I guess a travis-ci integration would also be a good idea. In the meantime i figured out that many haskell projects offer a nix build. I was suprised how easy it was to build a project with nix just |
@madnight Just a note, I've completed rewriting my functional tests from py.test to bats. Just wanted to let you know so no effort wasted. I've pasted the file below if you want to have a look, I'll have to make a branch from your current and do a PR onto you to merge in a bit. As I've written it we can use it to test gitstatus.py too. Also, the Haskell version fails a small change. The "remote gone" test case. See issue #20, essentially sometimes Edit 2: PR made directly onto your fork of me. |
- Replicated to cover same tests from py.test.
Hask: Implement complete BATS functional tests
It seems as tough the "Initial repo" tests fails in the Haskell version, but that seems also be the case for Olivier's Haskell version. This is due to |
Yeah, I think we've officially crammed enough into this one PR. Mark it as skip for now and we'll review that later. The remote gone test is also skipped at this moment, that was an issue raised after started. I'll just make separate haskell issues for those two and take PRs when fixed. |
Okay, I added the bats tests to travis ci (only for haskell for now) and disabled the initial repo test until the related issus is fixed. |
Ok, I'll review this and merge unless I find anything else. |
@madnight Sorry for the late reply, been a bit busy around the home. I have done some testing and apart from the pre-existing issues we'll resolve after I found these small issues.
When not in a git repo and STDIN passed to Haskell binary (i.e. how zshrc setup), it prints:
When not in a git repo and no STDIN set: (blank)
|
"Sorry for the late reply" no problem at all, I'm absolutely not in a hurry to get this merged or so, it takes as long as it takes
|
@madnight Any progress on these two issues? |
not yet, but i have it on my TODO stack and i have holiday's soon : ) |
Okay Issue 2. is resolved the mysterious (.tix) is generated by Haskell Program Coverage (hpc) which I enabled via compiler flags in an earlier commit. By the way in the meantime I learned how to build Haskell projects with nix, to me it is a much simpler and cleaner approach. I added a .nix build file, which is optional, people can still build with stack if they want. It might be that you still have trouble with .tix files, that is due to stack's build cache, Now I have a look at the first Issue. |
Okay I simply exit when not in a git repo, which should fix Issue 1. The Python script and the Haskell binary have the same output now.
|
This looks like it's ready to merge. I'm testing it locally and haven't run into any issues. Thanks! |
@starcraftman are you still trying to maintain this fork? |
@bruce-ricard It doesn't seem like that. Are you particularly interested in the Haskell version? |
I've since switched to https://github.com/woefe/git-prompt.zsh |
Not really no. Just interested in a maintained version. I think I'll give a try to the one @rnc linked. Thanks! |
This commit adds new features as described in #5
Currently i build and run it with:
stack build && stack install && src/.bin/gitstatus
Example outputs:
master 0 0 0 0 0 2 0 0 origin/master 0 0
:e4d4dee 0 0 6 1 0 2 0 0 .. 0 1/1
What's missing:
Let's discuss this : )