hsenv icon indicating copy to clipboard operation
hsenv copied to clipboard

Let the cabal build "dist" directory alone

Open adinapoli opened this issue 12 years ago • 18 comments

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

adinapoli avatar Sep 13 '12 12:09 adinapoli

The problem lies in Snap making an assumption about the buildDir. Look at this fix in pandoc for example: https://github.com/jgm/pandoc/commit/ff0061281f23b730034aeeb5de0568d6eec08b32

dudebout avatar Jan 16 '13 15:01 dudebout

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 :)

adinapoli avatar Jan 16 '13 15:01 adinapoli

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.

dudebout avatar Jan 16 '13 15:01 dudebout

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.

adinapoli avatar Jan 17 '13 08:01 adinapoli

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.

tmhedberg avatar Jan 17 '13 13:01 tmhedberg

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 :(

adinapoli avatar Jan 17 '13 13:01 adinapoli

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.

dudebout avatar Jan 17 '13 13:01 dudebout

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.

tmhedberg avatar Jan 17 '13 15:01 tmhedberg

+1

adinapoli avatar Jan 17 '13 15:01 adinapoli

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: https://github.com/Paczesiowa/virthualenv/issues/34 and it does not feel that it was done in an optimal way.

dudebout avatar Jan 17 '13 15:01 dudebout

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 :)

adinapoli avatar Jan 17 '13 15:01 adinapoli

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?

dudebout avatar Jan 17 '13 16:01 dudebout

@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.

tmhedberg avatar Jan 18 '13 00:01 tmhedberg

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.

dudebout avatar Jan 18 '13 01:01 dudebout

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.

tmhedberg avatar Jan 18 '13 04:01 tmhedberg

Nice @tmhedberg , ty.

adinapoli avatar Jan 18 '13 08:01 adinapoli

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

tmhedberg avatar Jan 18 '13 12:01 tmhedberg

So cheers @dudebout :beers:

adinapoli avatar Jan 18 '13 12:01 adinapoli