Skip to content
This repository has been archived by the owner on Oct 15, 2024. It is now read-only.

problems in mount.sh #4031

Closed
6 tasks
markus2330 opened this issue Sep 10, 2021 · 11 comments
Closed
6 tasks

problems in mount.sh #4031

markus2330 opened this issue Sep 10, 2021 · 11 comments

Comments

@markus2330
Copy link
Contributor

  • TODO in line 4
  • line 14: try to unmount if mounted (not needed for spec-mount, as it already overwrites previous mountpoints)
  • make sure notification is working? redshift: reloading #4030
  • add safety checks for presence of kdb, sudo and so on
  • contains some brittle code (e.g. expect certain output of kdb mount)
  • use argument instead of APP_PATH?
@markus2330
Copy link
Contributor Author

@qwepoizt Is this something you already looked into? Are there further improvements to be made?

@qwepoizt
Copy link
Contributor

No, actually I have not looked into this issue. I'm not aware of other improvements.

Do you want me to improve the script?

@markus2330
Copy link
Contributor Author

It is better if we get the issues, you are already working on, fully done. This issue can be easily reassigned as it is quite separate.

The main problem with the UX of the scripts is if they are executed when the mountpoints are already mounted:

The command kdb get terminated unsuccessfully with the info:
Invalid Keyname: keyname needs to start with /, meta:/, default:/, spec:/, proc:/, dir:/, user:/ or system:/ or maybe you tried to change a key that is already in a KeySet. Name was: '/elektra/mountpoints/spec:\/sw\/jonls\/redshift\/#0\/current/getplugins/#5#specload#specload#/config/file' 
Please report the issue at https://issues.libelektra.org/
The command kdb get terminated unsuccessfully with the info:
Invalid Keyname: keyname needs to start with /, meta:/, default:/, spec:/, proc:/, dir:/, user:/ or system:/ or maybe you tried to change a key that is already in a KeySet. Name was: '/elektra/mountpoints/spec:\/sw\/jonls\/redshift\/#0\/current/getplugins/#5#specload#specload#/config/app' 
Please report the issue at https://issues.libelektra.org/
ERROR: another mountpoint already exists on spec:/sw/jonls/redshift/#0/current. Please umount first.

So a lot of very misleading errors until the real problem is printed. It would be much better if the script simply umounts itself.

@qwepoizt
Copy link
Contributor

It is better if we get the issues, you are already working on, fully done. This issue can be easily reassigned as it is quite separate.

Okay, that sounds good to me!

It would be much better if the script simply umounts itself.

Good idea, I agree!

@stale
Copy link

stale bot commented Sep 28, 2022

I mark this issue stale as it did not have any activity for one year. I'll close it in two weeks if no further activity occurs. If you want it to be alive again, ping the issue by writing a message here or create a new issue with the remainder of this issue.
Thank you for your contributions 💖

@stale stale bot added the stale label Sep 28, 2022
@kodebach kodebach removed the stale label Sep 28, 2022
@kodebach
Copy link
Member

Eventually the generated mount.sh should probably be replaced by some C code that uses the new mount tools.

@markus2330
Copy link
Contributor Author

I agree that most of it should be directly in kdb mount but at least the sudo part is better to be a shell script.

@kodebach
Copy link
Member

kodebach commented Oct 8, 2022

I didn't mean make it part of kdb mount. I meant make it part of the generated code for the application, i.e. generate a mountConfiguration function. So that the application can provide e.g. a ./app install command or call the function on first start etc. Of course actually creating the mountpoints via that function will require root access in a standard installation for now, i.e. sudo ./app install. But with #1074 the application could create user:/ mountpoints.

@markus2330
Copy link
Contributor Author

Ok, let us wait for the new mount API/tool before fixing anything.

Copy link

I mark this stale as it did not have any activity for one year. I'll close it in two weeks if no further activity occurs. If you want it to be alive again, ping by writing a message here or create a new issue with the remainder of this issue.
Thank you for your contributions 💖

@github-actions github-actions bot added the stale label Dec 15, 2023
Copy link

I closed this now because it has been inactive for more than one year. If I closed it by mistake, please do not hesitate to reopen it or create a new issue with the remainder of this issue.
Thank you for your contributions 💖

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Dec 30, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants