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

src: move some code under comp/ #720

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

thoughtpolice
Copy link
Contributor

The commit descriptions for each commit contain most of the detail. But in short: I don't see why they're spread out elsewhere when they are de facto part of the comp codebase. So just move them there.

The top-level structure under `src/` is awkward and it's unclear why Parsec is
here. Why not `vendor/`? Ultimately though, I think the best place is actually
inside the source code, because it's not like anything else uses it and it's
`bsc`-owned code already in practice anyway.

Signed-off-by: Austin Seipp <[email protected]>
@thoughtpolice
Copy link
Contributor Author

thoughtpolice commented Jul 20, 2024

And once again I forget that I tried to do this before, though, only for Parsec: #34

But the effort to migrate to upstream Parsec has stalled out. It's been several years. Unless someone is working on it, let's just do it and take the small win for now — I think taking this commit would be good just to clean up the codebase. I have a lot of other thoughts improving the build process but I won't put them here.

So I think we should still just do these.

Similarly to Parsec, this code is practically only used in one spot, so it
might as well be maintained there. I don't even know what codebase HTcl is
a 'vendoring' *of* and it's dated to over 15 years ago at minimum, so it's
probably lost to history now... Just put it all together for clarity and ease
of development.

Signed-off-by: Austin Seipp <[email protected]>
@quark17
Copy link
Collaborator

quark17 commented Sep 5, 2024

Moving Parsec under comp is fine. But moving htcl doesn't seem correct to me. The Parsec directory is just Haskell files, and it can go under comp where other directories of Haskell files exist. The htcl directory, however, has a C source file and a Makefile for building a C library -- none of the directories under comp require calling make inside of them. The htcl directory also has an associated Haskell file, that imports the C library. I guess that Haskell file could be moved under comp, but since it's specific to the C file, I'm not sure it makes sense to separate them. The htcl directory is like the stp and yices directories in this way, as they also build libraries and have the associated Haskell files; and there, the Haskell file is more closely tied to the library -- Yices, for instance, has a v2.6 subdirectory with the associated Haskell, and if we added a v2.7 subdirectory, then it would contain it's version of the Yices Haskell files.

Instead of moving things under src/comp/ (which is a misnamed directory anyway), can we instead make src/ the root directory for building? (Any makefiles or cabal files or whatnot can go in src/ instead of src/comp/, and maybe even rename comp.) Then, everything in src/vendor/ and src/comp/ would be under the root (src/) -- and I assume the reason for moving things under src/comp/ is about putting the Haskell files under a single build root (which is currently src/comp/)? Or, if I'm mistaken, can you explain what are the constraints of your build tool that is leading you to this PR, so that I can try to propose an alternative that meets those constraints?

I guess, since src/ has other directories unrelated to the Haskell builds, maybe the thing to do is to keep src/comp/ as the root for building the Haskell tools, and:

  1. Create a new directory src/comp/src_Haskell/ and move all the Haskell source files (and subdirectories containing Haskell source files) currently under src/comp/ to be under that subdirectory now.
  2. Move src/vendor/* to src/comp/*
  3. Update src/comp/Makefile to do the building in the vendor subdirectories (instead of src/Makefile doing it)

I notice that src/Makefile turns off parallel builds for comp and beyond, but allows parallel build for the vendor directories. That would be need to be reconsidered somehow -- otherwise the above change would force the vendor libraries to build sequentially (where they can be built in parallel currently).

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