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

Delete cabal.project #613

Merged
merged 5 commits into from
Sep 24, 2023
Merged

Delete cabal.project #613

merged 5 commits into from
Sep 24, 2023

Conversation

clyring
Copy link
Member

@clyring clyring commented Sep 16, 2023

It seems to no longer do anything useful, and it also prevents CI from detecting that include/bytestring-cpp-macros.h was not getting picked up by cabal sdist.

It seems to no longer do anything useful, and it also prevents
CI from detecting that include/bytestring-cpp-macros.h
was not getting picked up by `cabal sdist`.
In particular, this ensures that `cabal sdist` grabs it.
@clyring
Copy link
Member Author

clyring commented Sep 16, 2023

The cabal package description docs do not provide me much clarity on the relevant-looking package description fields includes and install-includes. But includes: bytestring-cpp-macros.h doesn't result in cabal sdist picking up the file while install-includes: bytestring-cpp-macros.h does.

"Note that in order for files listed in install-includes to be usable when compiling the package itself, they need to be listed in the includes field as well."

This doesn't appear to be true? Or is it a fluke that this appears to work for me?

This is in contrast to install-includes, which lists header files that are intended to be exposed to other packages that transitively depend on this library.

These macros are probably not very useful to expose to downstream users.

Maybe I should just use extra-source-files instead.

@Bodigrim
Copy link
Contributor

unix includes headers such as HsUnix.h three (!) times:

  • extra-source-files,
  • includes
  • install-includes

It does not harm, better be safe than sorry ;)

The cabal documentation claims as a pre-requisite for its
usability "when compiling the package itself."  It's unclear
if this is really true, but there is no harm done in adding
bytestring-cpp-macros.h there just in case.
@clyring
Copy link
Member Author

clyring commented Sep 17, 2023

CI seems to say there is, in fact, harm done.

@clyring
Copy link
Member Author

clyring commented Sep 17, 2023

I see this in the cabal build -v3 output:

/usr/bin/gcc returned ExitFailure 1 with error message:
In file included from /tmp/846-14.c:2:
./include/bytestring-cpp-macros.h:2:2: error: #error "bytestring-cpp-macros.h
does not work in C code yet"
2 | #error "bytestring-cpp-macros.h does not work in C code yet"
| ^~~~~

It seems that cabal-install believes bytestring-cpp-macros.h is broken because it is not meant for inclusion in C code.

It seems this macro is never visible in Haskell code.
But with real unaligned primops in the near future,
it doesn't seem worth messing with.
@Bodigrim Bodigrim merged commit f5c5c51 into haskell:master Sep 24, 2023
24 checks passed
clyring added a commit to clyring/bytestring that referenced this pull request Nov 16, 2023
The windows jobs were subtly broken by haskell#613, which caused
the build products to live in bytestring-*/dist-newstyle
instead of dist-newstyle.  This didn't appear to break
immediately because we restored cached stuff in dist-newstyle
including stale executables.  It wasn't until a week of
inactivity caused our cache to expire that the breakage
became appropriately obvious.

I don't really understand what went wrong with the centos job;
I just borrowed the workaround from haskell/text#541.
@clyring clyring mentioned this pull request Nov 16, 2023
Bodigrim pushed a commit that referenced this pull request Nov 16, 2023
The windows jobs were subtly broken by #613, which caused
the build products to live in bytestring-*/dist-newstyle
instead of dist-newstyle.  This didn't appear to break
immediately because we restored cached stuff in dist-newstyle
including stale executables.  It wasn't until a week of
inactivity caused our cache to expire that the breakage
became appropriately obvious.

I don't really understand what went wrong with the centos job;
I just borrowed the workaround from haskell/text#541.
clyring added a commit that referenced this pull request Jan 30, 2024
The windows jobs were subtly broken by #613, which caused
the build products to live in bytestring-*/dist-newstyle
instead of dist-newstyle.  This didn't appear to break
immediately because we restored cached stuff in dist-newstyle
including stale executables.  It wasn't until a week of
inactivity caused our cache to expire that the breakage
became appropriately obvious.

I don't really understand what went wrong with the centos job;
I just borrowed the workaround from haskell/text#541.

(cherry picked from commit bf5f5da)
@clyring clyring added this to the 0.12.1.0 milestone Jan 30, 2024
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

Successfully merging this pull request may close these issues.

2 participants