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

The shell action in v2.0.5 not working as before (v1.10.1) #174

Open
andreoliwa opened this issue Feb 9, 2022 · 22 comments
Open

The shell action in v2.0.5 not working as before (v1.10.1) #174

andreoliwa opened this issue Feb 9, 2022 · 22 comments

Comments

@andreoliwa
Copy link

Hey, it's me again.
Sorry for being annoying, I hope you don't mind my bug reports. 😬
I'm trying to post one issue at a time, the blockers first... 😅


I'm running the latest version from GitHub (pipx install git+https://github.com/tfeldmann/organize.git).

The shell is not behaving the same way it did before.

This is my rule, converted from v1.10.1 and enhanced with the new syntax from v2.0.5:

rules:
  - name: "Remove some hidden files and then remove empty dirs"
    locations: ~/
    filters:
      - name:
          startswith: .bash_history
    actions:
      - shell:
          cmd: "fd -uu -0 -tf -i .DS_Store ~/Downloads ~/Desktop ~/OneDrive ~/Documents | xargs -0 rm -v"
          ignore_errors: true
      - echo: "{shell.output}"
      - shell:
          cmd: "fd -uu -0 -tf -i .nomedia ~/Downloads ~/Desktop ~/OneDrive ~/Documents | xargs -0 rm -v"
          ignore_errors: true
      - echo: "{shell.output}"
      - shell:
          cmd: "find -f ~/Downloads -f ~/Desktop -f ~/Documents ~/OneDrive -mindepth 1 -type d -empty -print -delete"
          ignore_errors: true
      - echo: "{shell.output}"

This is the output:

❯ organize run
organize 2.0.5
Config: "/Users/aa/Library/Application Support/organize/config.yaml"

⚙ Remove some hidden files and then remove empty dirs ───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
/Users/aa
  .bash_history
    - (shell) $ fd -uu -0 -tf -i .DS_Store ~/Downloads ~/Desktop ~/OneDrive ~/Documents | xargs -0 rm -v
    - (echo) error: Found argument '-v' which wasn't expected, or isn't valid in this context
    - (echo)
    - (echo) USAGE:
    - (echo)     fd [FLAGS/OPTIONS] [<pattern>] [<path>...]
    - (echo)
    - (echo) For more information try --help
    - (shell) $ fd -uu -0 -tf -i .nomedia ~/Downloads ~/Desktop ~/OneDrive ~/Documents | xargs -0 rm -v
    - (echo) error: Found argument '-v' which wasn't expected, or isn't valid in this context
    - (echo)
    - (echo) USAGE:
    - (echo)     fd [FLAGS/OPTIONS] [<pattern>] [<path>...]
    - (echo)
    - (echo) For more information try --help
    - (shell) $ find -f ~/Downloads -f ~/Desktop -f ~/Documents ~/OneDrive -mindepth 1 -type d -empty -print -delete
    - (echo) find: ~/Downloads: No such file or directory
    - (echo) find: ~/Desktop: No such file or directory
    - (echo) find: ~/Documents: No such file or directory
    - (echo) find: ~/OneDrive: No such file or directory

success 1 / fail 0

If I run the command manually, exactly as in the rule, it works:

❯ fd -uu -0 -tf -i .DS_Store ~/Downloads ~/Desktop ~/OneDrive ~/Documents | xargs -0 rm -v
/Users/aa/Desktop/.DS_Store
/Users/aa/OneDrive/.DS_Store

Somehow shell expansion is not working, the ~/ path is not being expanded.
All these directories exist, despite the error in the output:

    - (echo) find: ~/Downloads: No such file or directory
    - (echo) find: ~/Desktop: No such file or directory
    - (echo) find: ~/Documents: No such file or directory
    - (echo) find: ~/OneDrive: No such file or directory
@andreoliwa
Copy link
Author

UPDATE: I tried using the full path instead of ~/.
It kind of works, but commands with pipes (|) still fail.

⚙ Remove some hidden files and then remove empty dirs ───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
/Users/aa
  .bash_history
    - (shell) $ fd -uu -0 -tf -i .DS_Store /Users/aa/Downloads /Users/aa/Desktop /Users/aa/OneDrive /Users/aa/Documents | xargs -0 rm -v
    - (echo) error: Found argument '-v' which wasn't expected, or isn't valid in this context
    - (echo)
    - (echo) USAGE:
    - (echo)     fd [FLAGS/OPTIONS] [<pattern>] [<path>...]
    - (echo)
    - (echo) For more information try --help
    - (shell) $ fd -uu -0 -tf -i .nomedia /Users/aa/Downloads /Users/aa/Desktop /Users/aa/OneDrive /Users/aa/Documents | xargs -0 rm -v
    - (echo) error: Found argument '-v' which wasn't expected, or isn't valid in this context
    - (echo)
    - (echo) USAGE:
    - (echo)     fd [FLAGS/OPTIONS] [<pattern>] [<path>...]
    - (echo)
    - (echo) For more information try --help
    - (shell) $ find -f /Users/aa/Downloads -f /Users/aa/Desktop -f /Users/aa/Documents /Users/aa/OneDrive -mindepth 1 -type d -empty -print -delete
    - (echo) /Users/aa/OneDrive/Pictures/Screenshots
    - (echo) /Users/aa/OneDrive/Pictures/WhatsApp Animated Gifs

I didn't want to hardcode /Users/<username>.
I run organize with different users but the same config.yaml.

@rajscode
Copy link

rajscode commented Feb 9, 2022

@andreoliwa - You might be able to use the environment variable of $USER instead of ~

Such as the two examples below:
- echo: "/Users/{env.USER}/"

or

actions:
  - move:
      dest: /Users/{env.USER}/Downloads/Media/ 

@tfeldmann
Copy link
Owner

This should be fixed in v2.0.6. Can you try it?

@tfeldmann
Copy link
Owner

And thank you for the bug reports! They really help 👍

@tfeldmann
Copy link
Owner

Oops, got a bug in there! Please wait before testing

@tfeldmann
Copy link
Owner

Ok, please test with 2.0.7.

@andreoliwa
Copy link
Author

Yeah, something was fishy... good thing I used echo when I saw it was stuck on the "organizing" message.
I think it was about to remove all my files (or maybe it already removed some 😱 ):

⚙ Remove some hidden files and then remove empty dirs ───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
/Users/aa
  .bash_history
    - (shell) $ fd -uu -0 -tf -i .DS_Store ~/Downloads ~/Desktop ~/OneDrive ~/Documents | xargs -0 echo rm -v
    - (echo) ./Downloads
    - (echo) ./Documents
    - (echo) ./Movies
    - (echo) ./Applications
    - (echo) ./DataGripProjects
    - (echo) ./Postman
    - (echo) ./Code
    - (echo) ./Library
    - (echo) ./Desktop
    - (echo) ./Public
    - (echo) ./Pictures
    - (echo) ./OneDrive
    - (echo) ./Applications/JetBrains Toolbox
    (...)

@tfeldmann
Copy link
Owner

Yep that went very wrong!! I hope nothing's lost?

@tfeldmann
Copy link
Owner

Please wait a moment, I'm getting this sorted out.

@andreoliwa
Copy link
Author

I quickly checked the files, I didn't find anything missing yet. 😌

I'm getting a different error now.
I saw this error yesterday, don't know how to reproduce it:

"ERROR! 'shell' is undefined"

❯ organize sim
organize 2.0.7
Config: "/Users/aa/Library/Application Support/organize/config.yaml"
Warning: root path '/Users/aa/OneDrive/Pictures/Facebook' does not exist
Warning: root path '/Users/aa/OneDrive/Pictures/WhatsApp Animated Gifs' does not exist
Warning: root path '/Users/aa/OneDrive/Pictures/Samsung Gallery' does not exist
Warning: root path '/Users/aa/OneDrive/Pictures/Facebook' does not exist
Warning: root path '/Users/aa/OneDrive/Pictures/WhatsApp Animated Gifs' does not exist
Warning: root path '/Users/aa/OneDrive/Pictures/Samsung Gallery' does not exist
Warning: root path '/Users/aa/OneDrive/Pictures/Facebook' does not exist
Warning: root path '/Users/aa/OneDrive/Pictures/WhatsApp Animated Gifs' does not exist
Warning: root path '/Users/aa/OneDrive/Pictures/Samsung Gallery' does not exist
Warning: root path '/Users/aa/OneDrive/Pictures/Facebook' does not exist
Warning: root path '/Users/aa/OneDrive/Pictures/WhatsApp Animated Gifs' does not exist
Warning: root path '/Users/aa/OneDrive/Pictures/Samsung Gallery' does not exist
Warning: root path '/Users/aa/OneDrive/Pictures/Classify' does not exist
Warning: root path '/Users/aa/OneDrive/Pictures/Organize' does not exist

╭───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╮
│ SIMULATION                                                                                                                                                                                                        │
╰───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯

⚙ Remove some hidden files and then remove empty dirs ───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
/Users/aa
  .bash_history
    - (shell) $ fd -uu -0 -tf -i .DS_Store ~/Downloads ~/Desktop ~/OneDrive ~/Documents | xargs -0 echo rm -v
    - (echo) ERROR! 'shell' is undefined

@tfeldmann
Copy link
Owner

I'm so glad to hear that! Which python version are you on?

@tfeldmann
Copy link
Owner

Please try the 2.0.8. 2.0.7 is broken

@andreoliwa
Copy link
Author

I upgraded to 2.0.8.

I'm still getting the same 'shell' is undefined" error, as if the shell` action is not being called. 🤔

⚙ Remove some hidden files and then remove empty dirs ───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
/Users/aa
  .bash_history
    - (shell) $ fd -uu -0 -tf -i .DS_Store ~/Downloads ~/Desktop ~/OneDrive ~/Documents | xargs -0 echo rm -v
    - (echo) ERROR! 'shell' is undefined

Which python version are you on?

Python 3.9.9

❯ { pipx list --include-injected | rg -A 2 organize; python -V; }
   package organize-tool 2.0.8, Python 3.9.9
    - organize
    Injected Packages:
      - textract 1.6.4
Python 3.9.9

@tfeldmann
Copy link
Owner

That's strange. Can you post your full rule? Does this happen only in simulation?

@tfeldmann
Copy link
Owner

tfeldmann commented Feb 9, 2022

I could reproduce this with echo: "{shell.output}" in simulation. Because the shell command is not run the placeholder is not filled. I'm thinking about setting shell.output to ** simulation ** in this case?

@andreoliwa
Copy link
Author

Now it's working! 🎉

I's using the latest version from PyPI:

❯ organize --version
organize, version 2.0.8
❯ organize run
organize 2.0.8
Config: "/Users/aa/Library/Application Support/organize/config.yaml"

⚙ Remove some hidden files and then remove empty dirs ───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
/Users/aa
  .bash_history
    - (shell) $ fd -uu -0 -tf -i .DS_Store ~/Downloads ~/Desktop ~/OneDrive ~/Documents | xargs -0 rm -v
    - (echo) /Users/aa/Documents/.DS_Store
    - (echo) /Users/aa/OneDrive/Pictures/.DS_Store
    - (echo) /Users/aa/OneDrive/.DS_Store
    - (shell) $ fd -uu -0 -tf -i .nomedia ~/Downloads ~/Desktop ~/OneDrive ~/Documents | xargs -0 rm -v
    - (echo) /Users/aa/Downloads/.nomedia
    - (shell) $ find -f ~/Downloads -f ~/Desktop -f ~/Documents ~/OneDrive -mindepth 1 -type d -empty -print -delete
    - (echo) /Users/aa/Downloads/untitled folder

That's strange. Can you post your full rule? Does this happen only in simulation?

My bad, you're right... the ERROR! 'shell' is undefined only shows in simulation. 🙈

I'm thinking about setting shell.output to ** simulation ** in this case?

Yes, that is a good workaround. 👍🏻

@andreoliwa
Copy link
Author

andreoliwa commented Feb 9, 2022

One more thing: is there a way to show the current output of the shell command being executed instead of ⠴ organizing with the animated icon?

Now the output only shows after the command is over.
If it's a long-running command, I don't see what's going on.

If I remember well, it worked like this in v1.10.1.

I can also open a separate issue for this.

@tfeldmann
Copy link
Owner

I'm glad it's working now 👍 In the next version you'll be able to specify the value of {shell.output} in the simulation (** simulation ** by default).

Just a note: Are you deleting these .DS_Store for every file you find?

You can basically do something like this:

rules:
  - locations:
      - path: ~/Desktop/
        system_exclude_files: []
    subfolders: true
    filters:
      - name: .DS_Store
    actions:
      - trash

And as of v2, organize can now delete empty directories, too...

One more thing: is there a way to show the current output of the shell command being executed instead of ⠴ organizing with the animated icon?

This is how it should be, but I have to admit I didn't test with long running commands. I'll look into that!

@andreoliwa
Copy link
Author

Just a note: Are you deleting these .DS_Store for every file you find?
You can basically do something like this:
And as of v2, organize can now delete empty directories, too...

Yes, I tried the new syntax as well. 🙂

rules:
  - name: "Delete .DS_Store"
    locations:
      - path: ~/Documents
        system_exclude_files: []
      - path: ~/Downloads/
        system_exclude_files: []
      - path: ~/Desktop/
        system_exclude_files: []
      - path: ~/Library/Mobile Documents/com~apple~QuickTimePlayerX/Documents
        system_exclude_files: []
      - path: ~/OneDrive/
        system_exclude_files: []
    subfolders: true
    filters:
      - name: .DS_Store
    actions:
      - trash

I kept using the shell action to test it in v2.0.x.
And also because find is still a bit faster in this case (3 times)... 😅

I'm planning to rewrite or remove these shell actions.

but I have to admit I didn't test with long running commands. I'll look into that!

Thanks! 🙏🏻

And also many thanks for the super-fast answers, fixes and new releases! 💪🏻

@enricomarogna
Copy link

enricomarogna commented Apr 28, 2022

One more thing: is there a way to show the current output of the shell command being executed instead of ⠴ organizing with the animated icon?

Now the output only shows after the command is over. If it's a long-running command, I don't see what's going on.

If I remember well, it worked like this in v1.10.1.

I can also open a separate issue for this.

Quoto, in the previous version it was possible to see the shell output in real time and this allowed me to be able to act if my intervention was required. For example, among my commands I have brew upgrade, which allows you to update installed packages and it is possible that my intervention may be required, to confirm an action or to enter the root password. If this happens, now ⠴ organizing remains in the loop because it is not possible to intervene in the shell.

@fenilgmehta
Copy link

fenilgmehta commented Oct 2, 2022

How about giving the users an option to decide which shell they want to use to execute the shell commands ? Shell examples for Linux can be bash, zsh, etc.

Why am I saying so ?

As far as I have seen, Python automatically uses sh (on Linux) for executing commands in subprocess.run(...) (shell.py#L68) (and other similar functions/methods of subprocess) when shell=True. But, users generally write commands which are tested and run properly on bash (or some other shell) and not tried on sh. Thus resulting in unexpected outcomes at runtime.

Following is the console output when I used subprocess.run(..., shell=True)

>>> import subprocess
>>> subprocess.run('pstree -aps $$ ; ps', shell=True)
systemd,1 rhgb --switched-root --system --deserialize 31
  └─systemd,1075 --user
      └─plasmashell,1232 --no-respawn
          └─konsole,16832
              └─zsh,16849
                  └─python3,16919
                      └─sh,16952 -c pstree -aps $$ ; ps     👈 See this, sh -c "..." is used
                          └─pstree,16953 -aps 16952
    PID TTY          TIME CMD
  16849 pts/1    00:00:00 zsh
  16919 pts/1    00:00:00 python3
  16952 pts/1    00:00:00 ps
CompletedProcess(args='pstree -aps $$ ; ps', returncode=0)
>>> 

Secondly, giving the user an option to decide the shell will ensure determinism at runtime about which shell is being used to execute the commands they write in the config file and how they are interpreted. Example:

echo cool 1>> /dev/stderr &> /dev/null
# bash --> no output
# zsh  --> output="cool", unexpected behaviour is seen when redirecting stdout to stderr, and
#                         unexpected behaviour has been seen with few other commands as well
# sh   --> output="cool", the command is launched asynchronously due to misinterpretation of &>

@tfeldmann
Copy link
Owner

Valid points, but can't you do subprocess("bash somescript.sh", shell=True)?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants