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

Enable manual path entry in the new profile assistant #1119

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

tleedjarv
Copy link
Contributor

This is a complete fix for #1116. The GUI currently does not allow skipping the assistant (one can manually configure the profile more freely once past the assistant). This patch adds optional manual entry for sync paths so that more advanced users can get through it with less hassle.

@tleedjarv
Copy link
Contributor Author

(CI fails not related to the PR.)

@Drugoy
Copy link

Drugoy commented Feb 5, 2025

Did CI produce a build as an artifact? It looks like it did not.
Without it - I'm sorry, I can't test: this project has a build script from different millennium, which I hardly understand how to make it work:
I tried to follow the docs and execute just make (after installing some packages like ocaml and make [which I deduced are essential, but are not mentioned in the readme]), but got an error related to something macos (I am on linux):

ocamlopt: osxsupport.c ---> osxsupport.o
ocamlopt -g -I lwt -I ubase -I system -I system/generic -I lwt/generic -I +unix -I +str      -ccopt '-o '/root/gits/github/tleedjarv/unison/src/osxsupport.o -c /root/gits/github/tleedjarv/unison/src/osxsupport.c
In file included from /usr/lib/ocaml/caml/config.h:57,
                 from /usr/lib/ocaml/caml/mlvalues.h:22,
                 from /root/gits/github/tleedjarv/unison/src/osxsupport.c:4:
/usr/lib/gcc/x86_64-alpine-linux-musl/14.2.0/include/stdint.h:9:26: error: no include path in which to search for stdint.h
    9 | # include_next <stdint.h>
      |                          ^
In file included from /usr/lib/ocaml/caml/mlvalues.h:23:
/usr/lib/ocaml/caml/misc.h:29:10: fatal error: stdlib.h: No such file or directory
   29 | #include <stdlib.h>
      |          ^~~~~~~~~~
compilation terminated.
make[2]: *** [Makefile.OCaml:206: osxsupport.o] Error 2
make[2]: Leaving directory '/root/gits/github/tleedjarv/unison/src'
make[1]: *** [Makefile:42: all] Error 2
make[1]: Leaving directory '/root/gits/github/tleedjarv/unison/src'
make: *** [Makefile:22: src] Error 2

I then tried to figure out how to narrow the scope of stuff it attempts to build and decided that what I needed was just make gui and that way I got a different error:

# make gui
cd src && make gui
make[1]: Entering directory '/root/gits/github/tleedjarv/unison/src'
GUI library lablgtk not found. GTK GUI will not be built.
Not on macOS. macOS native GUI will not be built.

Building for Unix
NATIVE = true

make -f Makefile.OCaml gui
make[2]: Entering directory '/root/gits/github/tleedjarv/unison/src'
ocamlopt: pixmaps.ml ---> pixmaps.cmx
ocamlopt -g -I lwt -I ubase -I system -I system/generic -I lwt/generic -I +unix -I +str -I +lablgtk3 -I +cairo2   -c /root/gits/github/tleedjarv/unison/src/pixmaps.ml
File "/root/gits/github/tleedjarv/unison/src/pixmaps.ml", line 261, characters 2-31:
261 |   Gpointer.unsafe_create_region ~path:[|1|] ~get_length:(fun _ -> sz) buf
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Error: Unbound module Gpointer
make[2]: *** [Makefile.OCaml:202: pixmaps.cmx] Error 2
make[2]: Leaving directory '/root/gits/github/tleedjarv/unison/src'
make[1]: *** [Makefile:42: gui] Error 2
make[1]: Leaving directory '/root/gits/github/tleedjarv/unison/src'
make: *** [Makefile:28: gui] Error 2

I don't understand what's wrong here.
Just as some debug info how I got the above:

podman run -it --rm alpine sh

and then inside:

apk update
apk add ocaml git make
git clone --single-branch --branch fix-1116 'https://github.com/tleedjarv/unison.git' ~/gits/github/tleedjarv/unison
cd ~/gits/github/tleedjarv/unison/
make     # the first failed attempt
make gui # the second failed attempt

edit: I also tried building in a debian-based container, but got the same result.

@tleedjarv
Copy link
Contributor Author

Did CI produce a build as an artifact? It looks like it did not.

Yes, yes it did. Are you not able to see the artifacts? If you do not see them directly then you have to click on 'Summary' or on 'CI' (the link name is different depending on where you start from).

Without it - I'm sorry, I can't test: this project has a build script from different millennium, which I hardly understand how to make it work: I tried to follow the docs and execute just make (after installing some packages like ocaml and make [which I deduced are essential, but are not mentioned in the readme])

I fail to see why you'd need to understand the build script at all. You seemed to have followed the wrong docs if you missed the essentials; they're pretty hard to miss.

but got an error related to something macos (I am on linux):

No, not related to macos. The errors are about stdlib.h and stdint.h.

I then tried to figure out how to narrow the scope of stuff it attempts to build and decided that what I needed was just make gui and that way I got a different error:

The error is explained in the output, though. "GUI library lablgtk not found. GTK GUI will not be built."
You need to install ocaml-lablgtk3 (Alpine) or lablgtk3 (Debian-based). But it doesn't matter now, the build would still fail with the same error about stdlib.h and stdint.h as above. I have no clue what those are about.

apk update
apk add ocaml git make
git clone --single-branch --branch fix-1116 'https://github.com/tleedjarv/unison.git' ~/gits/github/tleedjarv/unison
cd ~/gits/github/tleedjarv/unison/
make     # the first failed attempt
make gui # the second failed attempt

edit: I also tried building in a debian-based container, but got the same result.

That should work (provided lablgtk3 is installed). The error you're seeing is not coming from Unison's code. It would be of great help if you could figure out why you're seeing the error you're seeing. Could it be an issue with gcc or musl installation or the environment? Are you seeing the same error in Debian, too, or did you refer to the missing lablgtk only?

@Drugoy
Copy link

Drugoy commented Feb 5, 2025

I feel a bit embarrassed, but I don't understand where to look for the artifact files.
image

@tleedjarv
Copy link
Contributor Author

There are two ways to get to the artifacts. Either click on any of the 'Details' links you see and then on 'Summary' on the left hand side. Or click on 'Checks' on the very top of the PR (above first comment) and then click on 'CI' on the left hand side.

GTK file chooser allows selecting either files or directories but not
both. File choosers in the new profile assistant are configured for
selecting directories, with the expectation that this covers the vast
majority of usages. This patch adds text entry fields for users who wish
to manually enter the paths. As a side effect, this enables specifying a
single file as a sync root.
@Drugoy
Copy link

Drugoy commented Feb 6, 2025

Thank you for educating me! I'd never find that myself.
I've tested - and functionally it now seems to work, but the wording (fields are called 'First directory' and 'Second directory' even when now they don't necessarily need to be directories) and visuals (a button that transforms the form by adding 2 text fields to the left from file pickers) are IMO weird:
image
But since it now indeed permits the user to use GUI to create a file-to-file sync - I am content with the patch.
Thank you!

@gdt
Copy link
Collaborator

gdt commented Feb 6, 2025

Are you suggesting

  • use "path" instead of directory?
  • when clicking "manually", remove the picker widgets?
    or do you mean something else?

@Drugoy
Copy link

Drugoy commented Feb 6, 2025

I'd suggest some wording like "file or directory".
As for the button - I'd suggest to:

  • remove that button completely
  • make the fields for the text paths always displayed and occupy the whole line
  • shrink the drop-down-list button into a a small square (like 24 x 24 px) icon, working as a button that simply opens a file picker, - that should be capable of selecting either files or directories
  • when the user confirms their choice in the file picker's window by hitting 'select' button - the selected element's path should simply get pasted into the corresponding text field

Another nitpick would be to limit the 2nd file picker's scope to the type (dirs XOR files) selected for the 1st field, to avoid a possible collision of user erroneously creating a file-to-dir sync.

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.

3 participants