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

Let the cabal build "dist" directory alone #32

Open
adinapoli opened this issue Sep 13, 2012 · 18 comments
Open

Let the cabal build "dist" directory alone #32

adinapoli opened this issue Sep 13, 2012 · 18 comments

Comments

@adinapoli
Copy link
Contributor

Hi,

I don't know if this is a specifc hsenv issue of is related to the way cabal works or is sandboxed. A common problem an hsenv user has is that cabal build puts its output in the dist directory. With hsenv activated instead, the fold is renamed dist_<env-name>.

The problem is that this breaks a lot of scripts (for example the ones Snap framework uses for running test coverage), because this scripts expect to find the dist directory, without the trailing suffix.

One solution is to symlink the dist_<env-name> to point to dist, but this solution is not elegant due to the fact it binds the script to the presence of hsenv. The process should be agnostic.

How to reproduce it:

  • Activate an environment and inside it go with cabal configure and cabal build

Expected Behaviour:

  • Files got compiled inside dist folder, regardless to the environment name

Real Behaviour:

  • Files are put inside a folder called dist_<env-name>

Cheers,
Alfredo

@dudebout
Copy link
Contributor

The problem lies in Snap making an assumption about the buildDir.
Look at this fix in pandoc for example: jgm/pandoc@ff00612

@adinapoli
Copy link
Contributor Author

probably, but still I can't see any good reason to change the dist directory name.
In fact, in my fork I've patched the code to fix this patch and everything is working as expected, so I can't see any possible drawback, only benefits :)

@dudebout
Copy link
Contributor

One dist directory per hsenv is the desired behavior. An hsenv is an isolated environment. If you create multiple hsenv and do different configurations in each hsenv you will expect different build results. It is not as simple but it is the only reasonable behavior.

@adinapoli
Copy link
Contributor Author

I see your point. From my point of view, since tipically a hsenv env is associate to one and only one project (aka folder) I don't expect to have multiple dist folders, but only one. I struggle to find an example where it could be useful to fireup two or more isolated environment for the same project, maybe to test different build versions?

I suspect that most depends on the expected behavior for the final user though.
For my pov I'm done because my local repo already fixes this, although it might be useful to know what other users think as well.

@tmhedberg
Copy link
Contributor

Would it be useful to have an option to pass to the hsenv command that causes it to symlink dist to dist_<hsenv name> for compatibility with 3rd-party tools that expect a dist directory? Then you could choose which route you wanted to take on a per-project basis. Of course, you could always create the link yourself, but it might be nice to have it happen automatically if it's something you commonly need.

@adinapoli
Copy link
Contributor Author

yes, it might work. The only nuisance is that if you invoke "cabal clean", as far as I remember, it wipes out the /dist directory, and so our symlink :(

@dudebout
Copy link
Contributor

What I think would be good for that most common case (i.e. having only one hsenv) is to restore the unnamed hsenv folder. I thought it was better when an invocation of hsenv without any argument was creating a .hsenv folder and usind dist. Having the option of multiple environments is a good step forward but I feel that the simple case got a little neglected by that move. I was never a big fan of having the name of my directory being appended to the .hsenv and dist folders.

@tmhedberg
Copy link
Contributor

How about leaving the default behavior the way it is currently, but adding an option to organize the directories the old way? So using something like hsenv --simple would create the directory as .hsenv, build using dist, etc.

I agree with you about the older way being more commonly useful, but I would be reluctant to change the default behavior that other users may now be relying on.

@adinapoli
Copy link
Contributor Author

+1

@dudebout
Copy link
Contributor

I think the new behavior is counter intuitive when you use the other equivalent tools such as virtualenv for python. You should have to use a flag only when you want a special behavior. As @adinapoli pointed, the new name for dist is not the desired default behavior and it is fairly rare that one person wants multiple hsenvs. When you want multiple hsenvs then playing with the options is not much of a problem.

The new behavior was put in hsenv only because of an issue: Paczesiowa/virthualenv#34 and it does not feel that it was done in an optimal way.

@adinapoli
Copy link
Contributor Author

I think that, if we remove the dist SUFFIX whilst retaining the hsenv_<env_name> we can have the cake and eat it too: this means that the user will still have MULTIPLE environments (because has multiple hsenv_* folders) but everytime he build he will create one and only one dist dir. It's unlikely that the final user bothers about not having separate dist directiories, as long as at least an executable gets build! :)
Hope I've express the idea clearly :)

@dudebout
Copy link
Contributor

The default naming of .hsenv_<name_of_containing_dir> is still a little weird. What is so special about the name of the containing dir?

@tmhedberg
Copy link
Contributor

@adinapoli I don't think we can get away with multiple .hsenv_* directories and a single dist directory. Cabal, as far as I understand it, does not always treat dist as write-only; in some cases it may change its behavior depending on the contents of dist. This means that a single dist should not be shared across sandboxes. Either we need to allow only a single sandbox per project directory (the way hsenv used to work), or keep the current behavior.

@dudebout:

I think we can probably change the default behavior back to the old, single-sandbox way of doing things, as long as we provide an option to keep the current behavior for those who need it.

The name of the containing directory is only special in that it is assumed to be the name of the project. Even in the current version, you can explicitly specify a suffix for your hsenv when initializing it on the command line.

@dudebout
Copy link
Contributor

This is exactly what I think we should do. And keep the --name option. This way when you do not give a name there is no name given instead of the directory.

@tmhedberg
Copy link
Contributor

This issue has now been addressed in my fork of hsenv. The default behavior when initializing a new sandbox is to omit the suffix from the .hsenv and dist directories. You can still add a suffix explicitly if you want one: if you use hsenv --name=foo, then the suffix _foo will be added.

@adinapoli
Copy link
Contributor Author

Nice @tmhedberg , ty.

@tmhedberg
Copy link
Contributor

@dudebout gets the credit. He wrote the patch. I just merged it. :)

@adinapoli
Copy link
Contributor Author

So cheers @dudebout 🍻

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

No branches or pull requests

3 participants