Skip to content

JS: add support for utimes/lutimes/futimes #285

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Aug 20, 2023
Merged

JS: add support for utimes/lutimes/futimes #285

merged 1 commit into from
Aug 20, 2023

Conversation

hsyl20
Copy link
Contributor

@hsyl20 hsyl20 commented Jul 4, 2023

In particular, add support for touchFd required here: https://gitlab.haskell.org/ghc/ghc/-/merge_requests/9237

Cc @bgamari

@hasufell
Copy link
Member

hasufell commented Jul 4, 2023

How can we test this in CI?

@hsyl20 hsyl20 marked this pull request as draft July 4, 2023 11:39
@hsyl20
Copy link
Contributor Author

hsyl20 commented Jul 4, 2023

You need to build GHC with the JS backend enabled. Apparently CI is using ghcup so it would be good to fix haskell/ghcup-hs#838 first

@Bodigrim
Copy link
Contributor

Bodigrim commented Jul 4, 2023

@hsyl20 it's fine to add a custom CI job, which manually grabs GHC JS from elsewhere, without ghcup, if it's easier. But I'm reluctant to merge without one.

@hsyl20
Copy link
Contributor Author

hsyl20 commented Jul 5, 2023

it's fine to add a custom CI job, which manually grabs GHC JS from elsewhere, without ghcup, if it's easier. But I'm reluctant to merge without one.

We're in the process of fixing GHCup to build cross-compilers. Moritz, however, suggested that we use the haskell.nix infrastructure to avoid the build times. I'll try this.

@hasufell
Copy link
Member

hasufell commented Jul 6, 2023

Moritz, however, suggested that we use the haskell.nix infrastructure to avoid the build times. I'll try this.

I personally don't want to maintain nix code in CI.

GHCup should be fixed to be able to install cross toolchain from bindist as well.

@hsyl20
Copy link
Contributor Author

hsyl20 commented Jul 6, 2023

I personally don't want to maintain nix code in CI.

Nix would only be used to provide the environment with the appropriate ghc (nix shell some_url). Does that count as nix code? :)

GHCup should be fixed to be able to install cross toolchain from bindist as well.

Sadly upstream doesn't provide JS bindists. Does ghcup already provide custom bindists (and where are they hosted)?

@hasufell
Copy link
Member

hasufell commented Jul 6, 2023

Nix would only be used to provide the environment with the appropriate ghc (nix shell some_url). Does that count as nix code? :)

Yes, my experience with nix has been rather negative and I want to avoid pinging other people whenever there's a problem with it (and IME, there are a lot).

Sadly upstream doesn't provide JS bindists.

We can just build our own.

@bgamari
Copy link
Contributor

bgamari commented Jul 7, 2023 •
edited
Loading

I have incorporated this change into ghc!9237 which passed CI.

@Bodigrim
Copy link
Contributor

Bodigrim commented Jul 7, 2023

@bgamari the reason we insist on GitHub CI job is so that the feature does not get accidentally broken in the very next commit. We cannot really tell all future contributors "go bump a submodule in GHC source tree and check that it works, then report back".

@hasufell
Copy link
Member

hasufell commented Jul 7, 2023

ghcup is very close to being able to install ghc JS cross from bindist. I already built one: https://downloads.haskell.org/~ghcup/unofficial-bindists/ghc/javascript-unknown-ghcjs-9.6.2/

It'll require another ghcup release first. Then we'll maintain a separate ghcup metadata for JS.

hsyl20 added a commit to hsyl20/unix that referenced this pull request Jul 19, 2023
@hsyl20
Fix building and testing with the JS backend
833b051 
Note that all tests have been disabled because none of them passes due
to missing C function support (e.g. pipe). But it's a good basis to
start adding tests for supported things (e.g. touchFd, cf haskell#285).

This requires a recent version of GHC that provides
getMonotonicTimeNSec. See
https://gitlab.haskell.org/ghc/ghc/-/issues/23687 and
https://gitlab.haskell.org/ghc/ghc/-/merge_requests/10900.
hsyl20 added a commit to hsyl20/unix that referenced this pull request Jul 19, 2023
@hsyl20
Fix building and testing with the JS backend
ec18bc6 
Note that most tests have been disabled because they don't pass due
to missing C function support (e.g. pipe). But it's a good basis to
start adding tests for supported things (e.g. touchFd, cf haskell#285).

This requires a recent version of GHC that provides
getMonotonicTimeNSec. See
https://gitlab.haskell.org/ghc/ghc/-/issues/23687 and
https://gitlab.haskell.org/ghc/ghc/-/merge_requests/10900.
hsyl20 added a commit to hsyl20/unix that referenced this pull request Jul 19, 2023
@hsyl20
Fix building and testing with the JS backend
7443f9c 
Note that most tests have been disabled because they don't pass due
to missing C function support (e.g. pipe). But it's a good basis to
start adding tests for supported things (e.g. touchFd, cf haskell#285).

This requires a recent version of GHC that provides
getMonotonicTimeNSec. See
https://gitlab.haskell.org/ghc/ghc/-/issues/23687 and
https://gitlab.haskell.org/ghc/ghc/-/merge_requests/10900.
@hsyl20 hsyl20 mentioned this pull request Jul 19, 2023
Merged
@hsyl20
Copy link
Contributor Author

hsyl20 commented Jul 19, 2023

I've opened #290 to make the testsuite runnable with GHC head+!10900 built with the JS backend. I just have to rebase and to add a test for this PR.

CI will be more difficult because we need to use GHC 9.8 at least. But releasing 9.8 might be blocked on this...

@hasufell
Copy link
Member

Can't we just build a bindist from the current 9.8 branch?

I already have ghcup CI that tests ghcup compile ghc against JS cross: https://github.com/haskell/ghcup-hs/actions/runs/5593656559

I don't think CI is prerequisite to get this merged, but it needs to remain a TODO list, so that we don't regress.

@hsyl20 hsyl20 mentioned this pull request Jul 19, 2023
Closed
@hsyl20
Copy link
Contributor Author

hsyl20 commented Jul 19, 2023

Can't we just build a bindist from the current 9.8 branch?

After https://gitlab.haskell.org/ghc/ghc/-/merge_requests/10900 is backported, we can.

I don't think CI is prerequisite to get this merged, but it needs to remain a TODO list, so that we don't regress.

I've opened #291 to track this.

@hsyl20
Copy link
Contributor Author

hsyl20 commented Jul 19, 2023

I'm working on a test for this PR but I need https://gitlab.haskell.org/ghc/ghc/-/merge_requests/10909 to fix createFile and https://gitlab.haskell.org/ghc/ghc/-/merge_requests/10912 to fix getFileStatus...

hasufell pushed a commit that referenced this pull request Jul 23, 2023
@hsyl20 @hasufell
Fix building and testing with the JS backend
48d590c 
Note that most tests have been disabled because they don't pass due
to missing C function support (e.g. pipe). But it's a good basis to
start adding tests for supported things (e.g. touchFd, cf #285).

This requires a recent version of GHC that provides
getMonotonicTimeNSec. See
https://gitlab.haskell.org/ghc/ghc/-/issues/23687 and
https://gitlab.haskell.org/ghc/ghc/-/merge_requests/10900.
@Bodigrim
Copy link
Contributor

@hsyl20 could you please rebase? How can I test this patch manually?

@hsyl20
Copy link
Contributor Author

hsyl20 commented Jul 24, 2023

@Bodigrim I'm on vacation this week but next week sure. I have a wip test but it isn't complete yet. There is also a test for touchFd in the ghc MR.

@hsyl20 hsyl20 marked this pull request as ready for review August 17, 2023 09:40
@hsyl20
Copy link
Contributor Author

hsyl20 commented Aug 17, 2023

I've rebased. Test is stil wip and depends on (at least) https://gitlab.haskell.org/ghc/ghc/-/merge_requests/10909 (currently being merged).

@Bodigrim Bodigrim merged commit 295369f into haskell:master Aug 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hasufell hasufell hasufell approved these changes

@Bodigrim Bodigrim Bodigrim approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@hsyl20 @hasufell @Bodigrim @bgamari