Skip to content

Add a Version_ module which does not depend on install dirs #3909

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
sophie-h opened this issue Sep 28, 2016 · 20 comments
Closed

Add a Version_ module which does not depend on install dirs #3909

sophie-h opened this issue Sep 28, 2016 · 20 comments

Comments

@sophie-h
Copy link

I am using Paths_ (version) to output the version number in my project. However, the

bindir, libdir, datadir, libexecdir, sysconfdir :: FilePath

is making the binary non-reproducible. If this section can't be changed due to compatibility issues, there should be a new reproducible Buildconstants_ or what ever API.

By the way: It would be useful to export name and synopsis too, since they often appear in --version or --help.

@ezyang
Copy link
Contributor

ezyang commented Sep 28, 2016

Well, we certainly can't delete it. What if we added a new Version_ module which contained the information you asked for?

@sophie-h
Copy link
Author

That would be great and totally do the job.

@23Skidoo 23Skidoo added this to the milestone Sep 29, 2016
@dcoutts
Copy link
Contributor

dcoutts commented Oct 1, 2016

is making the binary non-reproducible.

@qua-bla sorry if it's an obvious question, but why does that make it non-reproducible? Surely these install paths would be the same each time, if you're using the same configuration.

@sophie-h
Copy link
Author

sophie-h commented Oct 1, 2016

On Sat, Oct 1, 2016 at 2:26 AM, Duncan Coutts
[email protected] wrote:

Surely these install paths would be the same each time, if you're
using the same configuration.

Sure, but why should the config be the same? I don't want force the
build server to do stuff outside the sandbox.

The idea of reproducible builds is not that I can build it twice on
/my/ computer ;)

@ezyang
Copy link
Contributor

ezyang commented Oct 1, 2016

I think we'd be happy to take a PR on this. But before working on it, it would be worth to write the putative docs; i.e., what would the API of this module be.

@ezyang ezyang changed the title autogen/Paths_ is not reproducible Add a Version_ module which does not depend on install dirs Oct 1, 2016
@sophie-h
Copy link
Author

sophie-h commented Oct 1, 2016

It would be also possible to fix this via a CPP #ifdef in Paths_ that can be controlled via a cabal option (something like "enable-reproducibility"). The advantage would be that package maintainers can fix reproducibility issues without touching code and no new API would be introduced.

On the other hand the name Paths_ is odd anyways and replacing Paths_ with Version_ as a package maintainer is doable.

Any thoughts?

@ezyang
Copy link
Contributor

ezyang commented Oct 1, 2016

I'm aesthetically opposed to putting CPP in Paths but I don't have any technical reasons for it (besides, I guess, CPP is bad).

@hvr
Copy link
Member

hvr commented Oct 2, 2016

but if we had a Paths_ w/o installdirs, then how would a package find where cabal installed its package data and other stuff?

@ezyang
Copy link
Contributor

ezyang commented Oct 2, 2016

Well, the point is anyone who would disable the install paths bits wouldn't actually use it (it can't be disabled by default.)

@hvr
Copy link
Member

hvr commented Oct 2, 2016

maybe we should have a new field/setting in .cabal to declare whether a package needs any paths, as then we could automatically leave them off for all packages inside an install-plan which don't need them.

@sophie-h
Copy link
Author

sophie-h commented Oct 2, 2016

I'm aesthetically opposed to putting CPP in Paths

totally

but I don't have any technical reasons for it (besides, I guess, CPP is bad).

There would be a new config options or something that would have to be carried around for ever. Sound like higher support cost to me. Also changing the flags in my small test implementation required a strange build-clean-build sequence to get the "actual" reproducible result. (There will be a good reason, but who wants to deal with that.)

How about

module ProgramInfo_test
  ( ProgramInfo
  , programInfo
  , programName
  , programSynopsis
  , programVersion
  , programCopyright
  , programHomepage
  ) where

import Data.Version (Version, makeVersion)

data ProgramInfo = ProgramInfo
  { programName :: String
  , programVersion :: Version
  , programSynopsis :: String
  , programCopyright :: String
  , programHomepage :: String
  }

programInfo :: ProgramInfo
programInfo =
  ProgramInfo
  { programName = "test"
  , programVersion = makeVersion [0, 1, 0, 0]
  , programSynopsis = "Example project"
  , programCopyright = "(c) 2016 Me"
  , programHomepage = "https://example.org"
  }

@ezyang
Copy link
Contributor

ezyang commented Oct 3, 2016

Is there any particular reason you want it to be a record, rather than just a series of top-level declarations?

@sophie-h
Copy link
Author

sophie-h commented Oct 3, 2016

Yes. It could be used as an argument to a function that generates a
--version or usage text. But that would require that the type
ProgramInfo is defined elsewhere.

@ezyang
Copy link
Contributor

ezyang commented Oct 4, 2016

Right, and I think that is a price too far to pay (you could just pass the arguments individually.)

@dcoutts
Copy link
Contributor

dcoutts commented Oct 12, 2016

Sure, but why should the config be the same? I don't want force the build server to do stuff outside the sandbox. The idea of reproducible builds is not that I can build it twice on /my/ computer ;)

@qua-bla Ok, I think this is an artefact of the current sandbox impl, but not anything fundamental. The configuration for the paths is about where the program/lib will be installed to. This ought to be the same between different machines.

Think about it this way: for some arbitrary C project, you would not expect that:

./configure --prefix=/opt/this

vs

./configure --prefix=/opt/that

would produce identical binaries. Rather you would expect that to reproduce a build you should use the same install locations. Sure, it shouldn't matter what temp build dir you use, but where you configure the thing to end up does matter. This is the same thing that's going on with Cabal, the --prefix and install dirs.

You shouldn't be saying that to be reproducible programs must not use any data files or otherwise know where they live. That's perfectly ok and not a problem for reproducibility.

The problem you're seeing is that the current cabal sandbox mechansim defaults to setting the prefix to be inside the sandbox. That is the problem, not the Paths module.

That said, there are other good reasons to split and extend the generated modules. Split so the paths only has the paths, and extended so there's more generated info available. So I welcome that, but don't confuse it with reproducibility.

The other thing you may want to look at to improve reproducibility is to finish support for properly relocatable (aka prefix independent) packages. There's partial support for this already, but it needs pushing through to completion.

@qnikst
Copy link
Collaborator

qnikst commented Jun 9, 2018

I'll try to solve this issue

@domenkozar
Copy link
Collaborator

domenkozar commented May 7, 2019

I'm a bit surprised that for statically (haskell wise) linked executables don't eliminate (via split sections) these bindings that are not used at all in the code.

blackheaven added a commit to blackheaven/cabal that referenced this issue Oct 15, 2022
blackheaven added a commit to blackheaven/cabal that referenced this issue Oct 15, 2022
blackheaven added a commit to blackheaven/cabal that referenced this issue Oct 15, 2022
blackheaven added a commit to blackheaven/cabal that referenced this issue Oct 15, 2022
blackheaven added a commit to blackheaven/cabal that referenced this issue Oct 15, 2022
blackheaven added a commit to blackheaven/cabal that referenced this issue Oct 15, 2022
blackheaven added a commit to blackheaven/cabal that referenced this issue Oct 15, 2022
blackheaven added a commit to blackheaven/cabal that referenced this issue Oct 15, 2022
blackheaven added a commit to blackheaven/cabal that referenced this issue Oct 15, 2022
blackheaven added a commit to blackheaven/cabal that referenced this issue Oct 16, 2022
blackheaven added a commit to blackheaven/cabal that referenced this issue Oct 16, 2022
blackheaven added a commit to blackheaven/cabal that referenced this issue Oct 16, 2022
blackheaven added a commit to blackheaven/cabal that referenced this issue Oct 16, 2022
blackheaven added a commit to blackheaven/cabal that referenced this issue Oct 16, 2022
blackheaven added a commit to blackheaven/cabal that referenced this issue Oct 29, 2022
blackheaven added a commit to blackheaven/cabal that referenced this issue Nov 8, 2022
blackheaven added a commit to blackheaven/cabal that referenced this issue Nov 8, 2022
blackheaven added a commit to blackheaven/cabal that referenced this issue Nov 8, 2022
blackheaven added a commit to blackheaven/cabal that referenced this issue Nov 9, 2022
blackheaven added a commit to blackheaven/cabal that referenced this issue Nov 9, 2022
blackheaven added a commit to blackheaven/cabal that referenced this issue Dec 29, 2022
kokobd pushed a commit to blackheaven/cabal that referenced this issue Dec 31, 2022
mergify bot added a commit that referenced this issue Dec 31, 2022
@blackheaven
Copy link
Collaborator

Fixed by #8534

@wismill
Copy link
Collaborator

wismill commented Jan 13, 2023

If fixed completely, can this issue be closed?

@andreabedini
Copy link
Collaborator

As fas as I can tell, #8534 implemented everything requested here. Feel free to reopen if you think otherwise.

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