Fallback for Process.fork not being supported on Windows

[kyleplump]

Process.fork is only supported on *nix operating systems: https://ruby-doc.org/core-2.6.2/Process.html#method-c-fork

But, spawning new processes off of the main thread is still supported using spawn()

This fix handles fork not being implemented by falling back on the already used Open3.popen3 call, which itself wraps the native spawn function: https://github.com/ruby/open3/blob/master/lib/open3.rb#L533

Might not be the most performant solution, but at least gets everything up and running

Please lmk if you notice anything I should change, thanks!

[timriley]

Hi @kyleplump, thanks for having a go at this!

Your solution would work for the assets compile command, since each assets_command here is short-lived, so we can accept it running in serial, one command after the other for each slice.

However, I don't think it will work for the assets watch command, since these commands are long-lived: they never terminate until the user explicitly interrupts them. This means we can't have them run in serial, we really do need them to be in parallel.

Would you like to have a go at making this work?

[kyleplump]

Thanks for the pointers @timriley! I would love to have a go at this

My initial reaction would have been to create a new thread if the method is not implemented, and in that thread make the system_call. But, I've just learned about the existence of the GIL which throws a fun wrench in the mix :)

I would be happy to create a new thread to encapsulate this new process so it runs concurrently, or wrap the new process in a Ractor to give true parallelism. Although, my (albeit limited) understanding is that by using ractors we would lose thread safety. Do you have any opinion on this?

EDIT: ended up going with Threads. Ractors seem neat, but I think still too experimental for this use case. lmk if this code works!

[pcopissa]

The proposed fix does not work on a Windows platform: the rescue NotImplementedError block is never called.
The code below does not crash and hanami assets compile terminates without error. I don't have any feedback as whether it worked or not (no text output is produced):

def fork_child_assets_command(slice)
  if Process.respond_to?(:fork)
    Process.fork do
      cmd, *args = assets_command(slice)
      system_call.call(cmd, *args, out_prefix: "[#{slice.slice_name}] ")
    rescue Interrupt
      # When this has been interrupted (by the Signal.trap handler in #call), catch the
      # interrupt and exit cleanly, without showing the default full backtrace.
    end
  else
    Thread.new do
      cmd, *args = assets_command(slice)
      system_call.call(cmd, *args, out_prefix: "[#{slice.slice_name}] ")
    end
  end   
end

[timriley]

Thanks very much for testing this, @pcopissa! I was about to look at this today, and your work is very informative!

I don't have a windows machine to test on, unfortunately. Given you're already familiar with the code and the problem, are you willing to have a go at a fix yourself?

If you can do it before Monday, then we can include the fix in Hanami 2.2.0! I recognise that is short notice, so I'm more than willing to work with your own timeframes, and make a patch release whenever we have a fix ready.

[pcopissa]

Hi @timriley I appreciate your offer but the fact is that I am not familiar at all with Hanami code... I just read the error messages, searched the Github repo for similar issue (and fixes) and tried to apply them on the version I have installed... I do have a Windows 10 laptop though and I am trying to get a version that works with Ruby 3.0 (even though I have 3.2 at the moment). Hanami did fit the bill but now I have a hard time understanding which is the minimal Ruby version. In some places it says 3.0 and in some others 3.1.

…

Il 31/10/2024 05:42 CET Tim Riley ***@***.***> ha scritto: Thanks very much for testing this, @pcopissa https://github.com/pcopissa! I was about to look at this today, and your work is very informative! I don't have a windows machine to test on, unfortunately. Given you're already familiar with the code and the problem, are you willing to have a go at a fix yourself? If you can do it before Monday, then we can include the fix in Hanami 2.2.0! I recognise that is short notice, so I'm more than willing to work with your own timeframes, and make a patch release whenever we have a fix ready. — Reply to this email directly, view it on GitHub #221 (comment), or unsubscribe https://github.com/notifications/unsubscribe-auth/AKPVUS2YFRZEYTNCUOXBUQ3Z6GYKPAVCNFSM6AAAAABLVBEW4SVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDINBYHE4TSNBQGI. You are receiving this because you were mentioned.Message ID: ***@***.***>

[timriley]

No worries at all, @pcopissa :)

Hanami as of 2.1 has a minimum Ruby version of 3.0. When we release 2.2, it will have a minimum Ruby version of 3.1.

[timriley]

👋🏼 Hi @dsisnero! Since you've filed and fixed various Windows-related issues with Hanami, I wonder whether you might be up for giving this PR a test, and if it doesn't work, trying a few different approaches to address the issue? Let me know if this sounds possible for you, and if so, whether there's any more info you might need :) Thank you!

[kyleplump]

thanks for the tip @pcopissa ! I think your suggestion is better than the original fix:

• its seemingly more consistent
• it does not rely on a failure to provide the intended functionality

a much better solution, thank you 😃

@timriley i can test out this new solution in the next day or two on a windows machine and update the PR if necessary, so it can be included in the release? I would of course welcome any additional testing or suggestions from @dsisnero though!

[timriley]

@kyleplump If you could test this adjusted approach and verify it's working, that would be amazing! I can certainly still include it in next week's release.

Here's the things to look out for:

• In an app with actual asset files in multiple slices, running hanami assets watch will generate an initial set of compiled assets into public/assets/
• Then, modifying an asset file in one slice will see that change detected and the relevant file(s) in public/assets/ automatically updated
• After, modifying an asset file in the other slice will also see that change detected and the relevant file(s) in public/assets/ automatically updated
• All the while, the output of the hanami assets watch command is reporting these changes/rebuilds as they happen
• Hitting ctrl+c will cleanly exit the process

In other words, just make sure it works like it does on Mac/Linux 😄

Thank you!