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

Concerning behaviour when actions run bash commands #57258

Closed
2 tasks done
Isaacson33 opened this issue Apr 26, 2024 · 14 comments · Fixed by qgis/QGIS-Documentation#9056
Closed
2 tasks done

Concerning behaviour when actions run bash commands #57258

Isaacson33 opened this issue Apr 26, 2024 · 14 comments · Fixed by qgis/QGIS-Documentation#9056
Labels
Bug Either a bug report, or a bug fix. Let's hope for the latter! Feedback Waiting on the submitter for answers

Comments

@Isaacson33
Copy link

Isaacson33 commented Apr 26, 2024

What is the bug or the crash?

I've created an action which is;

mkdir -p [%directorypath_field%] && touch [%pathtonewtextfile_field%]

when I run the action the new directory is created and another new directory is created within that using the pathtonewtextfile_field.

I've set the action to capture output, copied the output, pasted it into the console and run it from there - it does exactly what it's supposed to do (create a directory, then a text file within it)

so when the exact output is run from the console it creates a directory and text file, when it's run from Qgis actions it creates a directory and a subdirectory.

Nothing more than an annoyance on it's own, but a massive flaw if Qgis is not faithfully running the script as written, it could lead to all sorts of serious breakages.

Steps to reproduce the issue

  1. Create or open any layer with at least two columns {directorypath_field and pathtonewtextfile_field}, in the first is an enquoted directory path, in the other is an enquoted full file path - to a text file within the first directory (I'm using .md, but .txt works too)
  2. create an action for that layer as mkdir -p [%directorypath_field%] && touch [%pathtonewtextfile_field%]
  3. run the action
  4. (optional) set the action to capture, copy the claimed command, paste it into a console and run it from there

Versions

<style type="text/css"> p, li { white-space: pre-wrap; } </style>
QGIS version 3.34.3-Prizren QGIS code branch Release 3.34
Qt version 5.15.12
Python version 3.11.6
Compiled against GDAL/OGR 3.8.2 Running against GDAL/OGR 3.8.4
PROJ version 9.3.1
EPSG Registry database version v10.098 (2023-11-24)
GEOS version 3.12.0-CAPI-1.18.0
Compiled against SQLite 3.45.0 Running against SQLite 3.45.1
Compiled against PDAL 2.6.2 Running against PDAL 2.6.3
PostgreSQL client version 16.1
SpatiaLite version 5.1.0
QWT version 6.2.0
QScintilla2 version 2.14.1
OS version Manjaro Linux
       
Active Python plugins
TomBio 3.4.2
pluginbuilder3 3.2.1
db_manager 0.1.20
processing 2.12.99
grassprovider 2.12.99
otbprovider 2.12.99
QGIS version 3.34.3-Prizren QGIS code branch [Release 3.34](https://github.com/qgis/QGIS/tree/release-3_34) Qt version 5.15.12 Python version 3.11.6 Compiled against GDAL/OGR 3.8.2 Running against GDAL/OGR 3.8.4 PROJ version 9.3.1 EPSG Registry database version v10.098 (2023-11-24) GEOS version 3.12.0-CAPI-1.18.0 Compiled against SQLite 3.45.0 Running against SQLite 3.45.1 Compiled against PDAL 2.6.2 Running against PDAL 2.6.3 PostgreSQL client version 16.1 SpatiaLite version 5.1.0 QWT version 6.2.0 QScintilla2 version 2.14.1 OS version Manjaro Linux

Active Python plugins
TomBio
3.4.2
pluginbuilder3
3.2.1
db_manager
0.1.20
processing
2.12.99
grassprovider
2.12.99
otbprovider
2.12.99

Supported QGIS version

  • I'm running a supported QGIS version according to the roadmap.

New profile

Additional context

Each command works as an action separately ie mkdir -p [%directorypath_field%] makes a directory and touch [%pathtonewtextfile_field%] creates a text file at that location if the directory exists

@Isaacson33 Isaacson33 added the Bug Either a bug report, or a bug fix. Let's hope for the latter! label Apr 26, 2024
@m-kuhn
Copy link
Member

m-kuhn commented Apr 26, 2024

This action starts a single process, not a shell script. See also https://stackoverflow.com/a/20901985

@m-kuhn m-kuhn added the Feedback Waiting on the submitter for answers label Apr 26, 2024
@Isaacson33
Copy link
Author

This action starts a single process, not a shell script. See also https://stackoverflow.com/a/20901985

I see what you mean, thanks. I was lead astray by the documentation which says

https://docs.qgis.org/3.34/en/docs/training_manual/create_vector_data/actions.html

you can use shell commands for any operating system

Indeed, a quick browse of my home folder reveals the directories "&&" and "touch".
mkdir -p [%directorypath_field%] && touch [%pathtonewtextfile_field%] interpreted as a single process would, of course, start mkdir and pass it the argument following, which in this case would be trying to create a directory called {directorypath_field}, a directory called {&&}, a directory called {touch}, and a directory called {pathtonewtextfile_field}

So the behaviour is fully explained, but the documentation may need adjusting? Or is it just my poor interpretation?

@m-kuhn
Copy link
Member

m-kuhn commented Apr 27, 2024

Indeed, thanks for pointing out.

Would you mind proposing a better wording in https://github.com/qgis/QGIS-Documentation/edit/master/docs/training_manual/create_vector_data/actions.rst ?

@Isaacson33
Copy link
Author

Would you mind proposing a better wording in https://github.com/qgis/QGIS-Documentation/edit/master/docs/training_manual/create_vector_data/actions.rst ?

No problem. I don't know how (I've never contributed directly to a Github project before), but I can work that out, it might take me while is all.

I'd suggest the wording only needs tweaking, had it said

you can use [single] shell commands for any operating system [but not bash scripts]

... that certainly would have clarified things for me personally

@m-kuhn
Copy link
Member

m-kuhn commented Apr 28, 2024

That's great!
It should be rather simple, the link provided above is directly editable in the browser and then you can just click your way through green buttons until you have opened a new "pull request". See also https://docs.qgis.org/3.34/en/docs/documentation_guidelines/first_contribution.html#alternative-1-use-the-edit-on-github-shortcut .

@Isaacson33
Copy link
Author

That's great! It should be rather simple, the link provided above is directly editable in the browser and then you can just click your way through green buttons until you have opened a new "pull request". See also https://docs.qgis.org/3.34/en/docs/documentation_guidelines/first_contribution.html#alternative-1-use-the-edit-on-github-shortcut .

Ah... it may be not quite so simple then because the link you provided just takes me to a page with

You need to fork this repository to propose changes.
Sorry, you’re not able to edit this repository directly—you need to fork it and propose your changes from there instead.

Whereas the guidelines you've linked to suggest an...

Edit on GitHub link at the top right of each page

... which, unless I'm being really dense (a possibility!), is just not there on the page your first link directs me to (one big "green button" though with 'Fork this repository' on it, which sounds more substantial an action than I was expecting)

I don't want to mess anything up, but also I'm aware you time is limited and guiding a noob through editing a simple text file is probably not the best use of it. Any help would be much appreciated, but also, I'm quite happy to just wade through the manuals until I get it...

I generally just 'tinker' on my own computer and learn that way, not quite such a sensible approach on a globally significant shared repository perhaps...

@m-kuhn
Copy link
Member

m-kuhn commented Apr 28, 2024

No worries really! We are always happy to enable people to help contribute and additional help on the documentation is very welcome !

Here's what you need to do: click on the "Fork this repository" button. This is a one-time action which creates a copy of the documentation for you on your github account.
There is no risk to mess anything up, as you will always be editing your copy and can just "propose" changes via a "pull request". Only once it has been reviewed, it will be integrated into the official documentation.

If you run into any blocking issues or uncertainties, don't hesitate to ask again.

Isaacson33 added a commit to Isaacson33/QGIS-Documentation that referenced this issue Apr 28, 2024
Just made a minor change to the wording here in response to 
qgis/QGIS#57258
making it clearer that it is 'processes' that can be called from shell, not just any command (eg boolean commands won't work)
@Isaacson33
Copy link
Author

Cool, thanks. I've done that (I think - either that or I've wiped the entire repository and you'll all have to start again!)

But on reading through, I think the bit that needs clarifying more is in the other section of 'Actions' here https://docs.qgis.org/3.34/en/docs/user_manual/working_with_vector/vector_properties.html#defining-actions

Where I'd like to add

Actions can only invoke processes via the shell, so booleen operators (such as &) will not work. To use multiple commands echo the full command set to bash bash -c or have the action run a bash script.

If that makes sense?

@m-kuhn
Copy link
Member

m-kuhn commented Apr 28, 2024

Good progress

I think it would make more sense to say that "Actions can invoke a single process with arguments" and then explain that for & or | etc. you will need to create a shell via bash or similar.

Btw, you can very well type this proposal and click your way through to a pull request. We (including you) can then still fine tune this until it's ready.

@Isaacson33
Copy link
Author

OK, I've made the changes I think will work.

I made them on the version in my repository (the 'forked' version), which, it turns out, has all the documentation in it, clicked on 'commit changes' but it's asking...

Commit directly to the master branch or
Create a new branch for this commit and start a pull request

...and I'm afraid I can't recall which I did last time I thought it was the 'pull request' one, but the 'master branch' is set as the default option...

@m-kuhn
Copy link
Member

m-kuhn commented Apr 28, 2024

Let's walk through this quickly together:

Here are all currently open pull requests listed: https://github.com/qgis/QGIS-Documentation/pulls
It looks like there is one from you already. This was probably started by you choosing the second option one hour ago.

You can click on it to see a discussion. You will there also notice a tab "Files changed". Within this, you will be able to continue editing your already open proposal.

image

Only if you want to start a completely new documentation change in the future, you will need to redo the workflow.

I hope that makes sense.

@m-kuhn
Copy link
Member

m-kuhn commented Apr 28, 2024

Actually, now that you say it: this is located in another file. It is possible to edit that file in the same pull request:

  • click on "Edit file" as explained above
  • Navigate to the other file
    image
  • Edit
  • Commit

... or create a completely new pull request

Isaacson33 added a commit to Isaacson33/QGIS-Documentation that referenced this issue Apr 28, 2024
Adding the same clarification on command line actions to the user manual as in qgis/QGIS#57258
@Isaacson33
Copy link
Author

Thanks, that's all quite clear. I've made the second pull request to just follow through the additional information into the user manual, which I think is all that's required.

I really appreciate you taking the time to provide guidance.

DelazJ pushed a commit to qgis/QGIS-Documentation that referenced this issue Apr 28, 2024
Just made a minor change to the wording here in response to 
qgis/QGIS#57258
making it clearer that it is 'processes' that can be called from shell, not just any command (eg boolean commands won't work)
qgis-bot pushed a commit to qgis/QGIS-Documentation that referenced this issue Apr 28, 2024
Just made a minor change to the wording here in response to 
qgis/QGIS#57258
making it clearer that it is 'processes' that can be called from shell, not just any command (eg boolean commands won't work)
DelazJ pushed a commit to qgis/QGIS-Documentation that referenced this issue Apr 28, 2024
Just made a minor change to the wording here in response to 
qgis/QGIS#57258
making it clearer that it is 'processes' that can be called from shell, not just any command (eg boolean commands won't work)
@m-kuhn
Copy link
Member

m-kuhn commented Apr 28, 2024

A pleasure, thank you

qgis-bot pushed a commit to qgis/QGIS-Documentation that referenced this issue Apr 28, 2024
Adding the same clarification on command line actions to the user manual as in qgis/QGIS#57258
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Either a bug report, or a bug fix. Let's hope for the latter! Feedback Waiting on the submitter for answers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants