Adding bounds to library with common stanza results in malformed cabal file

Given https://hackage.haskell.org/package/ede-0.3.2.0/revision/0.cabal, if I run

$ hackage-cli add-bound aeson '< 1.6' ede-0.3.2.0.cabal

, hackage-cli applies the following patch:

@@ -56,6 +56,8 @@ common base
   other-modules:    Paths_ede
 
 library
+  build-depends: aeson <1.6
+
   import:          base
   hs-source-dirs:  lib
   exposed-modules:

If I try to push this revision, the response is:

Warning: ede-0.3.2.0.cabal:61:3: Unknown field: "import"
Pushing "ede-0.3.2.0.cabal" (ede-0.3.2.0~0) [review-mode] ...
Hackage response was (after 0.622 secs):
================================================================================
Errors:

Cannot remove existing library dependency on &#39;base&#39; in library  component

================================================================================

cabal check reveals:

Warning: These warnings may cause trouble when distributing the package:
Warning: ede.cabal:61:3: Unknown field: import. Common stanza imports should
be at the top of the enclosing section
Warning: Hackage would reject this package.

cabal check says:

Common stanza imports should be at the top of the enclosing section

I'd say this is also an upstream problem;

import statements have to be the first thing in a stanza haskell/cabal#5564

it is questionable for a declarative language that cabal aims to be to demand a certain ordering of the fields. A reason to demand import to be at the top would be that it brings identifiers into scope that are used subsequently. But is this the case for a stanza import?

I agree that it is a surprising constraint. I have no idea about the reasons though.

The way .cabal file works (due decisions done long time ago)

common foo
  something: foo
  if flag(foo-flag)
    something: foo1
  else
    something: foo2

library
  header: headerVal
  if os(windows)
    something: windows

  import: common

  footer: footerVal
  if impl(ghc)
    something: ghc

would desugar into

library
  header: headerVal
  if os(windows)
    something: windows

  something: foo
  if flag(foo-flag)
    something: foo1
  else
    something: foo2

  footer: footerVal
  if impl(ghc)
    something: ghc

and interpret as if it were

library
  header: headerVal
  something: foo
  footer: footerVal

  if os(windows)
    something: windows

  if flag(foo-flag)
    something: foo1
  else
    something: foo2

  if impl(ghc)
    something: ghc

because conditionals are pushed back.

That would be fine if all fields contents were commutative (like build-depends),
but buildable isn't, for example, the last one wins.

Thus to avoid any confusion imports have to come first.

That was also a conservative change, it can be relaxed later.
It would be better to "fix" the push-back of conditionals first though.

That said, future relaxations won't help hackage-cli, as older spec cabal files still have to be dealt with.

To fix the issue, I think we can simply tweak the existing logic so the additional build-depends are inserted after any imports.

Here's the add-bound implementation:

hackage-cli/src/Main.hs

Lines 780 to 822 in fbf1967

    
           AddBound AddBoundOptions {..} -> forM_ optABFiles $ \fp -> do 
        
               old <- BS.readFile fp 
        
               -- idea is simple: 
        
               -- - .cabal is line oriented file 
        
               -- - find "library" section start 
        
               -- - bonus: look of an indentation used from the next field/section there 
        
               -- - insert data into a bytestring "manually" 
        
               fs <- either (exitFailureWith . show) return $ C.readFields old 
        
               (lin, indent) <- maybe 
        
                   (exitFailureWith $ "Cannot find library section in " ++ fp) 
        
                   return 
        
                   (findLibrarySection fs) 
        
               let msgLines  = map ("-- " ++) optABMessage 
        
                   bdLine    = "build-depends: " ++ C.prettyShow optABPackageName ++ " " ++ C.prettyShow optABVersionRange 
        
                   midLines  = [ BS8.pack $ replicate indent ' ' ++ l 
        
                               | l <- msgLines ++ [bdLine] 
        
                               ] ++ [""] -- also add an empty line separator 
        
                   (preLines, postLines) = splitAt lin $ BS8.lines old 
        
                   new = BS8.unlines (preLines ++ midLines ++ postLines) 
        
               -- sanity check 
        
               let oldGpd = parseGenericPackageDescription' old 
        
                   newGpd = parseGenericPackageDescription' new 
        
                   oldRange = extractRange oldGpd optABPackageName 
        
                   newRange = extractRange newGpd optABPackageName 
        
                   oldRange' = C.intersectVersionRanges oldRange optABVersionRange 
        
               unless (C.toVersionIntervals newRange == C.toVersionIntervals oldRange') $ 
        
                  exitFailureWith $ unwords 
        
                      [ "Edit failed, version ranges don't match: " 
        
                      , C.prettyShow oldRange 
        
                      , "&&" 
        
                      , C.prettyShow optABVersionRange 
        
                      , "=/=" 
        
                      , C.prettyShow newRange 
        
                      ] 
        
               -- write new version 
        
               BS.writeFile fp new

…and the function that finds the insertion position:

hackage-cli/src/Main.hs

Lines 441 to 449 in fbf1967

    
           findLibrarySection :: [C.Field C.Position] -> Maybe (Int, Int) 
        
           findLibrarySection [] = Nothing 
        
           findLibrarySection (C.Section (C.Name (C.Position row _) "library") [] fs : _) = 
        
               Just (row, findIndent fs) 
        
             where 
        
               findIndent [] = 4 
        
               findIndent (f : _) = case C.fieldAnn f of 
        
                   C.Position _ col -> pred col 
        
           findLibrarySection (_ : fs) = findLibrarySection fs

I've developed https://github.com/Bodigrim/cabal-add, capable to insert dependencies even in the presense of common stanzas.

@Bodigrim

@Bodigrim Kudos! I'll try it out next time I need to make revisions...

I don’t want it to look as a shameless plug, but how do we feel about making hackage-cli to use cabal-add API instead of manual updates? cabal-add is almost two years old now and I have not received any complaints about how it works.

I wouldn't object a reimplementation of the add-bound command.

Personally, I stopped using it, I am using wgrep in Emacs to do bulk-editing of cabal files, with much better control over the result than with add-bound.

Fixed in 0.3.0.0.

@Bodigrim

Thank you, @Bodigrim and @andreasabel! :)

Navigation Menu

Repository navigation

Adding bounds to library with common stanza results in malformed cabal file #26

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

	AddBound AddBoundOptions {..} -> forM_ optABFiles $ \fp -> do
	old <- BS.readFile fp

	-- idea is simple:
	-- - .cabal is line oriented file
	-- - find "library" section start
	-- - bonus: look of an indentation used from the next field/section there
	-- - insert data into a bytestring "manually"
	fs <- either (exitFailureWith . show) return $ C.readFields old
	(lin, indent) <- maybe
	(exitFailureWith $ "Cannot find library section in " ++ fp)
	return
	(findLibrarySection fs)

	let msgLines = map ("-- " ++) optABMessage
	bdLine = "build-depends: " ++ C.prettyShow optABPackageName ++ " " ++ C.prettyShow optABVersionRange
	midLines = [ BS8.pack $ replicate indent ' ' ++ l
	\| l <- msgLines ++ [bdLine]
	] ++ [""] -- also add an empty line separator
	(preLines, postLines) = splitAt lin $ BS8.lines old
	new = BS8.unlines (preLines ++ midLines ++ postLines)

	-- sanity check
	let oldGpd = parseGenericPackageDescription' old
	newGpd = parseGenericPackageDescription' new

	oldRange = extractRange oldGpd optABPackageName
	newRange = extractRange newGpd optABPackageName

	oldRange' = C.intersectVersionRanges oldRange optABVersionRange

	unless (C.toVersionIntervals newRange == C.toVersionIntervals oldRange') $
	exitFailureWith $ unwords
	[ "Edit failed, version ranges don't match: "
	, C.prettyShow oldRange
	, "&&"
	, C.prettyShow optABVersionRange
	, "=/="
	, C.prettyShow newRange
	]

	-- write new version
	BS.writeFile fp new

	findLibrarySection :: [C.Field C.Position] -> Maybe (Int, Int)
	findLibrarySection [] = Nothing
	findLibrarySection (C.Section (C.Name (C.Position row _) "library") [] fs : _) =
	Just (row, findIndent fs)
	where
	findIndent [] = 4
	findIndent (f : _) = case C.fieldAnn f of
	C.Position _ col -> pred col
	findLibrarySection (_ : fs) = findLibrarySection fs

Adding bounds to library with common stanza results in malformed cabal file #26

Description

Activity

andreasabel commented on Oct 12, 2021

sjakobi commented on Oct 12, 2021

phadej commented on Oct 12, 2021

sjakobi commented on Oct 12, 2021

Bodigrim commented on Oct 20, 2023

andreasabel commented on Oct 21, 2023

Bodigrim commented on Aug 30, 2025

andreasabel commented on Aug 31, 2025

andreasabel commented on Aug 23, 2026

sjakobi commented on Aug 23, 2026

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions