Skip to content

Check Include-dirs, extra-lib-dirs etc exist at configure time. #170

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

Closed
bos opened this issue May 24, 2012 · 10 comments
Closed

Check Include-dirs, extra-lib-dirs etc exist at configure time. #170

bos opened this issue May 24, 2012 · 10 comments

Comments

@bos
Copy link
Contributor

bos commented May 24, 2012

(Imported from Trac #177, reported by guest on 2007-11-12)

Trying to build the HDBC-PostGreSQL database driver[1] revealed some poor handling of pathames under Windows.

The package requires that the paths to postgre header and libraries files be added to the .cabal file. My initial attempt was the obvious:

include-dirs: C:\Program Files\PostgreSQL\8.2\include, C:\Program Files\PostgreSQL\8.2\include\server, .
extra-lib-dirs: C:\Program Files\PostgreSQL\8.2\include, C:\Program Files\PostgreSQL\8.2\include\server
However, the package would not build. The include files "pg_config.h" and some others were not found. Using single or double quotes did not help. Finally, I moved the installation to "C:\pgsql" and updated the .cabal file:
include-dirs: C:\pgsql\include, C:\pgsql\include\server, .
extra-lib-dirs: C:\pgsql\lib
And the package built. It seems the spaces in the first path caused the problem.

For reference, this it the configure output on my machine:

Configuring HDBC-postgresql-1.0.1.0...
configure: Dependency base-any: using base-2.1.1
configure: Dependency mtl-any: using mtl-1.0.1
configure: Dependency HDBC>=1.0.0: using HDBC-1.0.1
configure: Dependency parsec-any: using parsec-2.0
configure: Using install prefix: C:\Program Files
configure: Binaries installed in: C:\Program Files\Haskell\bin
configure: Libraries installed in: C:\Program Files\Haskell\HDBC-postgresql-1.0.1.0\ghc-6.6.1
configure: Private binaries installed in: C:\Program Files\HDBC-postgresql-1.0.1.0
configure: Data files installed in: C:\Program Files\Common Files\HDBC-postgresql-1.0.1.0
configure: Using compiler: c:\ghc\ghc-6.6.1\bin\ghc.exe
configure: Compiler flavor: GHC
configure: Compiler version: 6.6.1
configure: Using package tool: c:\ghc\ghc-6.6.1\bin\ghc-pkg.exe
configure: Using ar found on system at: c:\ghc\ghc-6.6.1\bin\ar.exe
configure: Using haddock found on system at: C:\haddock\haddock-0.8\haddock.exe
configure: No pfesetup found
configure: Using ranlib found on system at: c:\MinGW\bin\ranlib.exe
configure: Using runghc found on system at: c:\ghc\ghc-6.6.1\bin\runghc.exe
configure: Using runhugs found on system at: C:\Program Files\WinHugs\runhugs.exe
configure: No happy found
configure: No alex found
configure: Using hsc2hs: c:\ghc\ghc-6.6.1\bin\hsc2hs.exe
configure: No c2hs found
configure: No cpphs found
configure: No greencard found
Feel free to email me at jgbailey AT gmail DOT com for more info.

[1] Version 1.0.1.0 available at http://hackage.haskell.org/cgi-bin/hackage-scripts/package/HDBC-postgresql-1.0.1.0. HDBC is also required. I used 1.0.1, available at http://hackage.haskell.org/cgi-bin/hackage-scripts/package/HDBC-1.0.1.

@bos
Copy link
Contributor Author

bos commented May 24, 2012

(Imported comment by @dcoutts on 2007-11-12)

The solution is to use "" in the field, like:

include-dirs: "C:\Program Files\PostgreSQL\8.2\include", "C:\Program Files\PostgreSQL\8.2\include\server", .
extra-lib-dirs: "C:\Program Files\PostgreSQL\8.2\include", "C:\Program Files\PostgreSQL\8.2\include\server"
because spaces or ',' are allowed as field separators. Perhaps we should reconsider this or if we cannot change due to backwards compatability we could think about a warning. For example we might check if each directory exists and in the warning message mention that directory names with spaces have to use ""s.

@bos
Copy link
Contributor Author

bos commented May 24, 2012

(Imported comment by ross on 2007-11-13)

Yes, this is documented behaviour, and can't be changed because we need to preserve backwards compatibility for the .cabal file format. But I also think space separators are a good thing, both for human editing and machine-generated field values.

@bos
Copy link
Contributor Author

bos commented May 24, 2012

(Imported comment by @dcoutts on 2007-11-16)

I agree.

It would be nice if we could provide a helpful warning though. We do expect all these paths to exist at configure time, so we could check for them.

@bos
Copy link
Contributor Author

bos commented May 24, 2012

(Imported comment by @dcoutts on 2007-11-18)

It should be easy to check at configure time if all the dirs we will need actually exist. If they do not exist it's a good indication of a mistake. Should these be warnings or errors? To help with the misunderstanding in the original example perhaps the warning message should mention that paths with spaces need to use ""s.

@bos
Copy link
Contributor Author

bos commented May 24, 2012

(Imported comment by @dcoutts on 2008-01-21)

Actually it's not clear that we should warn, and it certainly should not be an error. Packages sometimes have to specify include dirs that exist on one machine and not another. What is important is not if the dirs exist or not, but if the include files and libraries can be found by searching them.

This would make it harder to detect when someone has accidentally left out "" around a path containing a space. Hmm.

@bos
Copy link
Contributor Author

bos commented May 24, 2012

(Imported comment by @dcoutts on 2008-01-29)

We now check that relative paths do exist, eg:

include-dir: include
But we do not check absolute paths. The rationale is that relative paths are part of the package so if they're missing then we have a problem, but absolute paths are on the system and these might be just a list of possible paths that may exist on some systems and not others.

Incidentally this would help in the original complaint

include-dirs: C:\Program Files\PostgreSQL\8.2\include, C:\Program Files\PostgreSQL\8.2\include\server, .
extra-lib-dirs: C:\Program Files\PostgreSQL\8.2\include, C:\Program Files\PostgreSQL\8.2\include\server
We would get a warning:
Warning: 'include-dirs: Files\PostgreSQL\8.2\include' directory does not exist.
Warning: 'include-dirs: Files\PostgreSQL\8.2\include\server' directory does not exist.
Warning: 'extra-lib-dirs: Files\PostgreSQL\8.2\include' directory does not exist.
Warning: 'extra-lib-dirs: Files\PostgreSQL\8.2\include\server' directory does not exist.

@bos
Copy link
Contributor Author

bos commented May 24, 2012

(Imported comment by guest on 2008-02-20)

See also:

@bos
Copy link
Contributor Author

bos commented May 24, 2012

(Imported comment by @dcoutts on 2008-03-25)

Module/file dependency analysis is not relevant (#15 and #159). That's a different kind of dependency.

@bos
Copy link
Contributor Author

bos commented May 24, 2012

(Imported comment by @dcoutts on 2008-03-25)

So I think the decision is that we should only require that relative paths exist, not absolute paths. Ticket #262 covers checking that libs, and headers can actually be found.

We should check that the configure flags --extra-lib-dirs=, --extra-include-dirs= do point to directories that actually exist.

The only other improvement we could make would be to somehow notice that joining two adjacent components of a directory field with a space form a valid absolute path that actually exists, when both components do not, and in such a case suggest that the user perhaps meant to use a path with an embedded space in which case they must use "" Haskell String syntax.

@tibbe
Copy link
Member

tibbe commented Jan 28, 2013

Since we now check relative paths we cover most of the issue. Unless someone really wants to implement the space-in-path warning heuristic, realistically this won't get done. Closing for now. Feel free to reopen if you want to work on this.

@tibbe tibbe closed this as completed Jan 28, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants