Skip to content

[Kindle NT] fix terminal and timesync#12765

Merged
Frenzie merged 16 commits into
koreader:masterfrom
mergen3107:kindle_nt_fix_terminal_and_timesync
Nov 23, 2024
Merged

[Kindle NT] fix terminal and timesync#12765
Frenzie merged 16 commits into
koreader:masterfrom
mergen3107:kindle_nt_fix_terminal_and_timesync

Conversation

@mergen3107

@mergen3107 mergen3107 commented Nov 21, 2024

Copy link
Copy Markdown
Contributor

Closes #12760
Credits go to @benoit-pierre (Thank you!)

@NiLuJe
Can you please give it a quick glance to make sure Kobo won't suffer from this?

I don't have any other devices to test, so please let me know if there are issues:

  • newer Kindles?
  • emulator
  • Android
  • Pocketbook
  • Cervantes
  • else?

This change is Reviewable

Comment thread plugins/timesync.koplugin/main.lua Outdated
@mergen3107

Copy link
Copy Markdown
Contributor Author

Tested on Kindle 4 NT, all good.

Comment thread frontend/util.lua
@mergen3107

Copy link
Copy Markdown
Contributor Author

Tested again on K4NT, all good

@mergen3107

Copy link
Copy Markdown
Contributor Author

Now both ash and sh can be used for terminal emulator shell

Comment thread plugins/terminal.koplugin/main.lua Outdated
forgot to remove the old implementation

Co-authored-by: Benoit Pierre <benoit.pierre@gmail.com>
@mergen3107

mergen3107 commented Nov 21, 2024

Copy link
Copy Markdown
Contributor Author

First macos check failed ¯\(ツ)/¯

Comment thread frontend/util.lua Outdated

function util.isExecutable(file)
local attrs = lfs.attributes(file)
return attrs and attrs.mode == "file" and os.execute("test -x " .. file) == 0

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that elsewhere it's written as attr

koreader/frontend/util.lua

Lines 793 to 799 in bb23ed0

local attr = lfs.attributes(path) or {}
if attr.mode == "directory" then
if f ~= "." and f ~= ".." then
scan(path)
end
elseif attr.mode == "file" or attr.mode == "link" then
cb(path, f, attr)

But my actual point, is there a particular reason not to do something like this?

local permissions = attr.permissions
return permissions:match("x") ~= nil

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The value will look like, for example: rwxr-xr-x

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know myself. @benoit-pierre can you please comment on this?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, this is meaningless without taking into account file ownership and the effective user and group id of the current process.

We have access to access :P. Let me check if if can export the other necessary defines beside F_OK and use that instead.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, this is meaningless

Emphasis on something like, of course.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!
I am currently working on GH from web browser, I'll try to make these diffs carefully by hand :D

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.
Can someone please bump base?
Or please show me show to do it from the browser?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@benoit-pierre

getcwd: cannot access parent directories

Do you think this is because Android, and you have no access to /bin, where from bash is trying to start?
https://stackoverflow.com/questions/29396928/error-retrieving-current-directory-getcwd-cannot-access-parent-directories

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bash is actually in /system/xbin/bash on that system. But yeah, I don't know.

Comment thread plugins/timesync.koplugin/main.lua Outdated
Comment thread plugins/timesync.koplugin/main.lua Outdated
@mergen3107

Copy link
Copy Markdown
Contributor Author

I synced this branch to prepare for merging.
I hope I didn't break it :D

Comment thread plugins/terminal.koplugin/main.lua Outdated
Comment thread plugins/terminal.koplugin/main.lua Outdated
Comment thread plugins/timesync.koplugin/main.lua
@mergen3107

Copy link
Copy Markdown
Contributor Author

Waiting on one more base-bump with this: koreader/koreader-base#1983 (comment)

Otherwise, ready for review (and merge)

@benoit-pierre

Copy link
Copy Markdown
Member

Waiting on one more base-bump with this: koreader/koreader-base#1983 (comment)

There's no need to wait for that since ffi/posix_h.lua is correct.

@mergen3107

Copy link
Copy Markdown
Contributor Author

Ready to merge?
I would like to test this on K4NT.

The thing is, I don't have my VM right now, I can't test base changes.
But I can test nightlies :D

@Frenzie Frenzie added this to the 2025.01 milestone Nov 23, 2024
Comment thread plugins/terminal.koplugin/main.lua Outdated

@NiLuJe NiLuJe left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No time to test much of anything right now, but I don't see anything that could cause issues on kobo, at a quick glance.

@Frenzie
Frenzie merged commit e95541f into koreader:master Nov 23, 2024
@mergen3107
mergen3107 deleted the kindle_nt_fix_terminal_and_timesync branch November 24, 2024 05:05
0xstillb pushed a commit to 0xstillb/koreader-thai that referenced this pull request May 9, 2026
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.

[Kindle NT] Terminal plugin - Shell is not executable (and no Time Sync)

5 participants