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

Replacement results in "Invalid cross-device link" #145

Closed
lukasjuhrich opened this issue Oct 5, 2024 · 9 comments · Fixed by #148 or #135 · May be fixed by #170
Closed

Replacement results in "Invalid cross-device link" #145

lukasjuhrich opened this issue Oct 5, 2024 · 9 comments · Fixed by #148 or #135 · May be fixed by #170

Comments

@lukasjuhrich
Copy link

As soon as a temporary file is involved, the temporary file seems to cause problems:

stdout / stderr

❯ srgn -vv --py methods 'x' --py function-calls 'super'
[2024-10-05T13:03:02.670500Z INFO  srgn] Launching app with args: Cli { scope: "x", shell: None, composable_actions: ComposableActions { replace: Some("super"), upper: false, lower: false, titlecase: false, normalize: false, german: false, symbols: false }, standalone_actions: StandaloneActions { delete: false, squeeze: false }, options: GlobalOptions { glob: None, fail_no_files: false, invert: false, literal_string: false, fail_any: false, fail_none: false, join_language_scopes: false, line_numbers: false, only_matching: false, hidden: false, gitignored: false, sorted: false, stdin_override_to: None, threads: None, additional_verbosity: 2 }, languages_scopes: LanguageScopes { c: None, csharp: None, go: None, hcl: None, python: Some(PythonScope { python: [Methods, FunctionCalls], python_query: [] }), rust: None, typescript: None }, german_options: GermanOptions { german_prefer_original: false, german_naive: false } }
[2024-10-05T13:03:02.678043Z INFO  srgn] Detected stdin as readable: false.
[2024-10-05T13:03:02.678058Z INFO  srgn] Will walk file tree, applying actions.
[2024-10-05T13:03:02.678191Z INFO  srgn] Will walk file tree using 4 thread(s), processing in arbitrary order, starting from: Ok("/home/lukas/code/10ag/pycroft")
tests/ldap_sync/action/test_execution.py
[2024-10-05T13:03:02.695424Z ERROR srgn] Error walking at /home/lukas/code/10ag/pycroft/tests/ldap_sync/action/test_execution.py due to: I/O error at path /tmp/srgnXapArn: Invalid cross-device link (os error 18)
[2024-10-05T13:03:02.695451Z ERROR srgn] Aborting walk for safety
pycroft/lib/mail.py
[2024-10-05T13:03:02.698403Z ERROR srgn] Error walking at /home/lukas/code/10ag/pycroft/pycroft/lib/mail.py due to: I/O error at path /tmp/srgnJHwDBV: Invalid cross-device link (os error 18)
[2024-10-05T13:03:02.698425Z ERROR srgn] Aborting walk for safety
Error: Error processing path: I/O error at path /tmp/srgnJHwDBV: Invalid cross-device link (os error 18)

FS setup

  • tmp: /tmp tmpfs tmpfs rw,nosuid,nodev,nr_inodes=1048576,inode64
  • working directory: /home /dev/mapper/vghdd-lvhome ext4 rw,relatime,data=ordered

How can I debug this further?

Copy link

issuedigger bot commented Oct 5, 2024

The most similar issues to this one are:

  1. Fix cloning static query on every query call #76 , with a similarity score of 0.93.
  2. 0.13.0 build failure #106 , with a similarity score of 0.91.
  3. build(deps): bump tempfile from 3.12.0 to 3.13.0 #141 , with a similarity score of 0.91.

@lukasjuhrich
Copy link
Author

Sorry @issuedigger, these don't seem to be related :-)

@alexpovel
Copy link
Owner

alexpovel commented Oct 5, 2024

Hi @lukasjuhrich , thanks for your report!

Apologies for @issuedigger, it's an experiment and safe to ignore.

To debug this further:

  • you could run it with -vvvv, which is trace and therefore maximum level of verbosity, then post that output again here, please?
  • I can look into reproducing the issue. What steps do I need to take? Is pycroft a repo I can fetch?

Initial thoughts:

  • let's confirm the command you invoke is doing what you hope it's doing. To reiterate:

    it is searching for (Python) function calls within methods, and replacing every character x it finds with the string super. It does so recursively. For that, it will walk and look at all Python files (either with .py suffix or with a python3 or python shebang) starting at /home/lukas/code/10ag/pycroft.

    I'm only mentioning this because the order of arguments you specify is unusual. It's legal and works, but for example I didn't see the x at first! It's easier to miss. The x does not apply to the --py methods part at all if that's what you're after.

  • it will then write out any new results to a temporary file, and replace those files more or less atomically (overwrite the original with the newly created temporary one). The temp should be /tmp/srgnXapArn etc. This step seems to fail. Is there some special setup involved on your machine regarding /tmp, beyond tmpfs?

My FS setup/OS is different from yours, but I've run srgn on a vanilla Ubuntu 24 machine and it worked fine. I'll try that again if you can help me with reproduction steps!

@alexpovel
Copy link
Owner

alexpovel commented Oct 5, 2024

Oh and to try and clear up any confusion more: you titled the issue "two scopes", but your command actually uses 3:

  • --py methods (only regard methods)
  • --py function-calls (only regard function calls within methods aka the previous language scope)
  • x (only regard the literal character x within all previous scopes); the fact that x was called before --py function-calls doesn't matter for this sort of ordering

It then also has one action:

  • super (replacement)

If this is not what you intended, do you feel UX could be better? Documentation? Some fail-safe errors or warnings which could've helped you?

@lukasjuhrich
Copy link
Author

Thank you very much for your insightful response! Indeed, the error is with replacing. As you conjectured, I did not intend to replace anything.

reproduction

We can simplify the example by creating a test.py with contents print(min(3, 5)).

❯ srgn --py function-calls min
test.py
1:print(min(3, 5))

❯ srgn --py function-calls min max
test.py
[2024-10-08T14:27:02.918687Z ERROR srgn] Error walking at /home/lukas/code/10ag/pycroft/test/test.py due to: I/O error at path /tmp/srgnvBNwyP: Invalid cross-device link (os error 18)
[2024-10-08T14:27:02.918705Z ERROR srgn] Aborting walk for safety
Error: Error processing path: I/O error at path /tmp/srgnvBNwyP: Invalid cross-device link (os error 18)
TRACE logs (not really revealling anything new)
❯ srgn -vvvv --py function-calls min max                                                                                                                                                                                                                       
[2024-10-08T14:27:53.884856Z INFO  srgn] Launching app with args: Cli { scope: "min", shell: None, composable_actions: ComposableActions { replace: Some("max"), upper: false, lower: false, titlecase: false, normalize: false, german: false, symbols: false 
}, standalone_actions: StandaloneActions { delete: false, squeeze: false }, options: GlobalOptions { glob: None, fail_no_files: false, invert: false, literal_string: false, fail_any: false, fail_none: false, join_language_scopes: false, line_numbers: fals
e, only_matching: false, hidden: false, gitignored: false, sorted: false, stdin_override_to: None, threads: None, additional_verbosity: 4 }, languages_scopes: LanguageScopes { c: None, csharp: None, go: None, hcl: None, python: Some(PythonScope { python: 
[FunctionCalls], python_query: [] }), rust: None, typescript: None }, german_options: GermanOptions { german_prefer_original: false, german_naive: false } }
[2024-10-08T14:27:53.884897Z DEBUG srgn] Assembling scopers.                                                                   
[2024-10-08T14:27:53.891979Z DEBUG srgn] Done assembling scopers.                                                      
[2024-10-08T14:27:53.892000Z DEBUG srgn] Assembling actions.                                                                   
[2024-10-08T14:27:53.892015Z DEBUG srgn] Loaded action: Replacement                                                                                                                                                                                            
[2024-10-08T14:27:53.892019Z DEBUG srgn] Done assembling actions.                                                                                                                                                                                              
[2024-10-08T14:27:53.892034Z INFO  srgn] Detected stdin as readable: false.                                                                                                                                                                                    
[2024-10-08T14:27:53.892038Z INFO  srgn] Will walk file tree, applying actions.                                                                                                                                                                                
[2024-10-08T14:27:53.892159Z INFO  srgn] Will walk file tree using 4 thread(s), processing in arbitrary order, starting from: Ok("/home/lukas/code/10ag/pycroft/test")
[2024-10-08T14:27:53.892823Z DEBUG ignore::gitignore] opened gitignore file: /home/lukas/code/10ag/pycroft/.ignore
[2024-10-08T14:27:53.892879Z DEBUG globset] built glob set; 0 literals, 1 basenames, 0 extensions, 0 prefixes, 0 suffixes, 0 required extensions, 0 regexes
[2024-10-08T14:27:53.892986Z DEBUG ignore::gitignore] opened gitignore file: /home/lukas/code/10ag/pycroft/.gitignore
[2024-10-08T14:27:53.893201Z DEBUG globset] glob converted to regex: Glob { glob: "**/*~", re: "(?-u)^(?:/?|.*/)[^/]*\\~$", opts: GlobOptions { case_insensitive: false, literal_separator: true, backslash_escape: true, empty_alternates: false }, tokens: To
kens([RecursivePrefix, ZeroOrMore, Literal('~')]) }
[2024-10-08T14:27:53.893221Z DEBUG globset] glob converted to regex: Glob { glob: ".idea/**/dataSources", re: "(?-u)^\\.idea(?:/|/.*/)dataSources$", opts: GlobOptions { case_insensitive: false, literal_separator: true, backslash_escape: true, empty_altern
ates: false }, tokens: Tokens([Literal('.'), Literal('i'), Literal('d'), Literal('e'), Literal('a'), RecursiveZeroOrMore, Literal('d'), Literal('a'), Literal('t'), Literal('a'), Literal('S'), Literal('o'), Literal('u'), Literal('r'), Literal('c'), Literal
('e'), Literal('s')]) }                                                                                                                                                                                                                                        
[2024-10-08T14:27:53.893237Z DEBUG globset] glob converted to regex: Glob { glob: "**/npm-debug.log*", re: "(?-u)^(?:/?|.*/)npm\\-debug\\.log[^/]*$", opts: GlobOptions { case_insensitive: false, literal_separator: true, backslash_escape: true, empty_alter
nates: false }, tokens: Tokens([RecursivePrefix, Literal('n'), Literal('p'), Literal('m'), Literal('-'), Literal('d'), Literal('e'), Literal('b'), Literal('u'), Literal('g'), Literal('.'), Literal('l'), Literal('o'), Literal('g'), ZeroOrMore]) }
[2024-10-08T14:27:53.893251Z DEBUG globset] glob converted to regex: Glob { glob: "**/yarn-debug.log*", re: "(?-u)^(?:/?|.*/)yarn\\-debug\\.log[^/]*$", opts: GlobOptions { case_insensitive: false, literal_separator: true, backslash_escape: true, empty_alt
ernates: false }, tokens: Tokens([RecursivePrefix, Literal('y'), Literal('a'), Literal('r'), Literal('n'), Literal('-'), Literal('d'), Literal('e'), Literal('b'), Literal('u'), Literal('g'), Literal('.'), Literal('l'), Literal('o'), Literal('g'), ZeroOrMo
re]) }    
[2024-10-08T14:27:53.893262Z DEBUG globset] glob converted to regex: Glob { glob: "**/yarn-error.log*", re: "(?-u)^(?:/?|.*/)yarn\\-error\\.log[^/]*$", opts: GlobOptions { case_insensitive: false, literal_separator: true, backslash_escape: true, empty_alt
ernates: false }, tokens: Tokens([RecursivePrefix, Literal('y'), Literal('a'), Literal('r'), Literal('n'), Literal('-'), Literal('e'), Literal('r'), Literal('r'), Literal('o'), Literal('r'), Literal('.'), Literal('l'), Literal('o'), Literal('g'), ZeroOrMo
re]) }
[2024-10-08T14:27:53.893290Z DEBUG globset] glob converted to regex: Glob { glob: "**/*.py[cod]", re: "(?-u)^(?:/?|.*/)[^/]*\\.py[cod]$", opts: GlobOptions { case_insensitive: false, literal_separator: true, backslash_escape: true, empty_alternates: false
 }, tokens: Tokens([RecursivePrefix, ZeroOrMore, Literal('.'), Literal('p'), Literal('y'), Class { negated: false, ranges: [('c', 'c'), ('o', 'o'), ('d', 'd')] }]) }
[2024-10-08T14:27:53.893317Z DEBUG globset] glob converted to regex: Glob { glob: "**/.coverage.*", re: "(?-u)^(?:/?|.*/)\\.coverage\\.[^/]*$", opts: GlobOptions { case_insensitive: false, literal_separator: true, backslash_escape: true, empty_alternates:
 false }, tokens: Tokens([RecursivePrefix, Literal('.'), Literal('c'), Literal('o'), Literal('v'), Literal('e'), Literal('r'), Literal('a'), Literal('g'), Literal('e'), Literal('.'), ZeroOrMore]) }
[2024-10-08T14:27:53.893341Z DEBUG globset] glob converted to regex: Glob { glob: "**/celerybeat-schedule.*", re: "(?-u)^(?:/?|.*/)celerybeat\\-schedule\\.[^/]*$", opts: GlobOptions { case_insensitive: false, literal_separator: true, backslash_escape: tru
e, empty_alternates: false }, tokens: Tokens([RecursivePrefix, Literal('c'), Literal('e'), Literal('l'), Literal('e'), Literal('r'), Literal('y'), Literal('b'), Literal('e'), Literal('a'), Literal('t'), Literal('-'), Literal('s'), Literal('c'), Literal('h
'), Literal('e'), Literal('d'), Literal('u'), Literal('l'), Literal('e'), Literal('.'), ZeroOrMore]) }
[2024-10-08T14:27:53.893365Z DEBUG globset] built glob set; 16 literals, 63 basenames, 14 extensions, 0 prefixes, 0 suffixes, 7 required extensions, 8 regexes
[2024-10-08T14:27:53.895993Z DEBUG ignore::gitignore] opened gitignore file: /home/lukas/code/10ag/pycroft/.git/info/exclude
[2024-10-08T14:27:53.896053Z TRACE srgn] Skipping path (not a file): "/home/lukas/code/10ag/pycroft/test"
[2024-10-08T14:27:53.896150Z TRACE srgn] Language scoper considers path 'test.py' valid: true
[2024-10-08T14:27:53.896156Z DEBUG srgn] Processing path: "test.py"
[2024-10-08T14:27:53.896169Z DEBUG srgn] Building view.
[2024-10-08T14:27:53.896175Z TRACE srgn::scoping::view] Exploding scopes: ROScopes([ROScope(In("print(min(3, 5))\n", None))])
[2024-10-08T14:27:53.896183Z TRACE srgn::scoping::view] Exploding scope: ROScope(In("print(min(3, 5))\n", None))
[2024-10-08T14:27:53.896190Z TRACE srgn::scoping::langs] Parsing into AST: "print(min(3, 5))\n"
[2024-10-08T14:27:53.896281Z DEBUG srgn::scoping::langs] S expression of parsed source code is: "(module (expression_statement (call function: (identifier) arguments: (argument_list (call function: (identifier) arguments: (argument_list (integer) (integer
)))))))"
[2024-10-08T14:27:53.896290Z TRACE srgn::scoping::langs] Running query: Query { ptr: 0x573921837b90, capture_names: ["function-name"], capture_quantifiers: [[One]], text_predicates: [[]], property_settings: [[]], property_predicates: [[]], general_predica
tes: [[]] }
[2024-10-08T14:27:53.896320Z DEBUG srgn::ranges] Merging ranges: Ranges { inner: [0..5, 6..9] }
[2024-10-08T14:27:53.896327Z DEBUG srgn::ranges] Merged ranges: Ranges { inner: [0..5, 6..9] }
[2024-10-08T14:27:53.896333Z TRACE srgn::scoping::langs] Querying yielded ranges: Ranges { inner: [0..5, 6..9] }
[2024-10-08T14:27:53.896341Z TRACE srgn::scoping::scope] Constructing scopes from raw ranges: [(0..5, None), (6..9, None)]
[2024-10-08T14:27:53.896351Z DEBUG srgn::scoping::scope] Scopes: [ROScope(In("print", None)), ROScope(Out("(")), ROScope(In("min", None)), ROScope(Out("(3, 5))\n"))]
[2024-10-08T14:27:53.896359Z TRACE srgn::scoping::view] Exploded scope, new scopes are: [ROScope(In("print", None)), ROScope(Out("(")), ROScope(In("min", None)), ROScope(Out("(3, 5))\n"))]
[2024-10-08T14:27:53.896367Z TRACE srgn::scoping::view] Done exploding scopes.
[2024-10-08T14:27:53.896376Z TRACE srgn::scoping::view] Exploding scopes: ROScopes([ROScope(In("print", None)), ROScope(Out("(")), ROScope(In("min", None)), ROScope(Out("(3, 5))\n"))])
[2024-10-08T14:27:53.896382Z TRACE srgn::scoping::view] Exploding scope: ROScope(In("print", None))
[2024-10-08T14:27:53.896391Z TRACE srgn::scoping::scope] Constructing scopes from raw ranges: []
[2024-10-08T14:27:53.896396Z DEBUG srgn::scoping::scope] Scopes: [ROScope(Out("print"))]
[2024-10-08T14:27:53.896401Z TRACE srgn::scoping::view] Exploded scope, new scopes are: [ROScope(Out("print"))]
[2024-10-08T14:27:53.896405Z TRACE srgn::scoping::view] Exploding scope: ROScope(Out("("))
[2024-10-08T14:27:53.896408Z TRACE srgn::scoping::view] Exploded scope, new scopes are: [ROScope(Out("print")), ROScope(Out("("))]
[2024-10-08T14:27:53.896413Z TRACE srgn::scoping::view] Exploding scope: ROScope(In("min", None))
[2024-10-08T14:27:53.896420Z TRACE srgn::scoping::scope] Constructing scopes from raw ranges: [(0..3, Some(CaptureGroups({Numbered(0): "min"})))]
[2024-10-08T14:27:53.896427Z DEBUG srgn::scoping::scope] Scopes: [ROScope(In("min", Some(CaptureGroups({Numbered(0): "min"}))))]
[2024-10-08T14:27:53.896433Z TRACE srgn::scoping::view] Exploded scope, new scopes are: [ROScope(Out("print")), ROScope(Out("(")), ROScope(In("min", Some(CaptureGroups({Numbered(0): "min"}))))]
[2024-10-08T14:27:53.896440Z TRACE srgn::scoping::view] Exploding scope: ROScope(Out("(3, 5))\n"))
[2024-10-08T14:27:53.896444Z TRACE srgn::scoping::view] Exploded scope, new scopes are: [ROScope(Out("print")), ROScope(Out("(")), ROScope(In("min", Some(CaptureGroups({Numbered(0): "min"})))), ROScope(Out("(3, 5))\n"))]
[2024-10-08T14:27:53.896451Z TRACE srgn::scoping::view] Done exploding scopes.
[2024-10-08T14:27:53.896455Z DEBUG srgn] Done building view: ScopedView { scopes: RWScopes([RWScope(Out("print")), RWScope(Out("(")), RWScope(In("min", Some(CaptureGroups({Numbered(0): "min"})))), RWScope(Out("(3, 5))\n"))]) }
[2024-10-08T14:27:53.896464Z DEBUG srgn] Applying actions to view.
[2024-10-08T14:27:53.896468Z DEBUG srgn::scoping::view] Appending 'print'
[2024-10-08T14:27:53.896473Z DEBUG srgn::scoping::view] Appending '('
[2024-10-08T14:27:53.896478Z DEBUG srgn::scoping::view] Mapping with context: Some(CaptureGroups({Numbered(0): "min"}))
[2024-10-08T14:27:53.896485Z DEBUG srgn::actions::replace] Available capture group variables: {Numbered(0): "min"}
[2024-10-08T14:27:53.896492Z TRACE srgn::actions::replace::variables] Injecting variables. Current output is: '', current state is Noop
[2024-10-08T14:27:53.896499Z TRACE srgn::actions::replace::variables] Injecting variables. Current output is: 'm', current state is Noop
[2024-10-08T14:27:53.896505Z TRACE srgn::actions::replace::variables] Injecting variables. Current output is: 'ma', current state is Noop
[2024-10-08T14:27:53.896511Z TRACE srgn::actions::replace::variables] Finished character iteration, output is 'max', state is Noop
[2024-10-08T14:27:53.896517Z TRACE srgn::actions::replace::variables] Done injecting variables, final output is 'max', final state is Noop
[2024-10-08T14:27:53.896523Z DEBUG srgn::scoping::view] Replacing 'min' with 'max'
[2024-10-08T14:27:53.896530Z DEBUG srgn::scoping::view] Appending '(3, 5))\n'
[2024-10-08T14:27:53.896536Z DEBUG srgn] Writing to destination.
[2024-10-08T14:27:53.896543Z DEBUG srgn] Done writing to destination.
test.py
[2024-10-08T14:27:53.896556Z DEBUG srgn] Got new file contents, writing to file: "test.py"
[2024-10-08T14:27:53.896589Z TRACE srgn] Writing to temporary file: "/tmp/srgna54Gb0"
[2024-10-08T14:27:53.896635Z ERROR srgn] Error walking at /home/lukas/code/10ag/pycroft/test/test.py due to: I/O error at path /tmp/srgna54Gb0: Invalid cross-device link (os error 18)
[2024-10-08T14:27:53.896648Z ERROR srgn] Aborting walk for safety
Error: Error processing path: I/O error at path /tmp/srgna54Gb0: Invalid cross-device link (os error 18)

However, strace -f !! reveals some more information (truncated to the relevant part):

[pid 2307447] ioctl(2, TCGETS, {c_iflag=ICRNL|IXON|IUTF8, c_oflag=NL0|CR0|TAB0|BS0|VT0|FF0|OPOST|ONLCR, c_cflag=B38400|CS8|CREAD, c_lflag=ISIG|ICANON|ECHO|ECHOE|ECHOK|IEXTEN|ECHOCTL|ECHOKE, ...}) = 0
[pid 2307447] write(2, "\33[90m[\33[0m2024-10-08T14:34:42.90"..., 118[2024-10-08T14:34:42.905065Z DEBUG srgn] Got new file contents, writing to file: "test.py"
) = 118                                                                                                                        
[pid 2307447] openat(AT_FDCWD, "/tmp/srgnmKcnCp", O_RDWR|O_CREAT|O_EXCL|O_CLOEXEC, 0600) = 3
[pid 2307447] ioctl(2, TCGETS, {c_iflag=ICRNL|IXON|IUTF8, c_oflag=NL0|CR0|TAB0|BS0|VT0|FF0|OPOST|ONLCR, c_cflag=B38400|CS8|CREAD, c_lflag=ISIG|ICANON|ECHO|ECHOE|ECHOK|IEXTEN|ECHOCTL|ECHOKE, ...}) = 0
[pid 2307447] write(2, "\33[90m[\33[0m2024-10-08T14:34:42.90"..., 113[2024-10-08T14:34:42.905232Z TRACE srgn] Writing to temporary file: "/tmp/srgnmKcnCp"
) = 113                                                                                                                        
[pid 2307447] write(3, "print(max(3, 5))\n", 17) = 17  
[pid 2307447] renameat(AT_FDCWD, "/tmp/srgnmKcnCp", AT_FDCWD, "test.py") = -1 EXDEV (Invalid cross-device link)
[pid 2307447] unlink("/tmp/srgnmKcnCp") = 0 
[pid 2307447] close(3)                  = 0
[pid 2307447] ioctl(2, TCGETS, {c_iflag=ICRNL|IXON|IUTF8, c_oflag=NL0|CR0|TAB0|BS0|VT0|FF0|OPOST|ONLCR, c_cflag=B38400|CS8|CREAD, c_lflag=ISIG|ICANON|ECHO|ECHOE|ECHOK|IEXTEN|ECHOCTL|ECHOKE, ...}) = 0
[pid 2307447] write(2, "\33[90m[\33[0m2024-10-08T14:34:42.90"..., 215 <unfinished ...>
[pid 2307450] <... clock_nanosleep resumed>0x776875bff5d0) = 0
[pid 2307449] <... clock_nanosleep resumed>0x77686fdff5d0) = 0
[2024-10-08T14:34:42.905502Z ERROR srgn] Error walking at /home/lukas/code/10ag/pycroft/test/test.py due to: I/O error at path /tmp/srgnmKcnCp: Invalid cross-device link (os error 18)
[pid 2307450] clock_nanosleep(CLOCK_REALTIME, 0, {tv_sec=0, tv_nsec=1000000},  <unfinished ...>
[pid 2307449] clock_nanosleep(CLOCK_REALTIME, 0, {tv_sec=0, tv_nsec=1000000},  <unfinished ...>                                                                                                                                                                
[pid 2307447] <... write resumed>)      = 215
[pid 2307448] <... clock_nanosleep resumed>0x776875fff5d0) = 0
[pid 2307447] ioctl(2, TCGETS <unfinished ...>
[pid 2307448] clock_nanosleep(CLOCK_REALTIME, 0, {tv_sec=0, tv_nsec=1000000},  <unfinished ...>
[pid 2307447] <... ioctl resumed>, {c_iflag=ICRNL|IXON|IUTF8, c_oflag=NL0|CR0|TAB0|BS0|VT0|FF0|OPOST|ONLCR, c_cflag=B38400|CS8|CREAD, c_lflag=ISIG|ICANON|ECHO|ECHOE|ECHOK|IEXTEN|ECHOCTL|ECHOKE, ...}) = 0
[pid 2307447] write(2, "\33[90m[\33[0m2024-10-08T14:34:42.90"..., 97[2024-10-08T14:34:42.905694Z ERROR srgn] Aborting walk for safety
) = 97

The renameat syscall error confirms that the rename is a problem, precisely because /tmp and <destination dir> lie on different file systems.
man 2 renameat confirms that this should not work:

ERRORS
       […]
       EXDEV  oldpath and newpath are not on the same mounted filesystem.  (Linux permits a filesystem to be mounted at multiple points, but rename() does not work across different mount points, even if the same filesystem is mounted on both.)

uname -a says Linux juhrilaptop 6.10.3-arch1-2 #1 SMP PREEMPT_DYNAMIC Tue, 06 Aug 2024 07:21:19 +0000 x86_64 GNU/Linux.
Are you sure you have a proper tmpfs in your environment, and not just a /tmp folder?

UX

I'm not sure how to solve this, but let me present what I think the problems are.

First, that I can "push forward" the search argument (in this case, x) is a surprise to me, because the scope arguments are order dependent. This causes me to think of the whole chain of arguments as a pipeline, and even though I think I've read the docs telling me otherwise, I was convinced that I was searching for method bodies containing x and then narrowing further.

Second, I think it is very dangerous to have a destructive operation so "close" to a nondestructive one. I can think of two scenarios by which one can switch from search to replace by accident:

  1. What I did here:
    • starting with --py methods --py function-calls super and
    • naively trying to add something between the two --py scopes to narrow it down
  2. Using a pipe like srgn … | less and accidentally removing or forgetting to type the pipe character
  3. Fat-fingering the return key and accidentally adding a letter after the command line

I tend to be afraid of tools which punish me for mistakes like that, because I tend to make many. I feel this is contrary to the "fast" and "light-weight" tool that srgn aims to be (and for me already is) if I have to be careful like that all the time; this is nontrivial cognitive load.

I can think of two options:

  1. a separate replace subcommand, ideally with an abbreviation like r, so I can use srgn r ….
  2. a --go flag you can place wherever you like, where omission would cause a "dry run" of the replacement.

Both require changing the default to "don't replace unless told to", which might require a deprecation period.

Re documentation, I think changing the semantics is more desirable than documenting the existing semantics better for the reasons I've outlined.

what I actually wanted to achieve

I guess this would bettor belong in a disussion instead of an issue, but since you asked, let me tell you of my use case: I wanted to find all the super() function calls inside methods without a certain decorator attached (@overload). I have not yet found out how to do so or whether srgn can do this.

@lukasjuhrich lukasjuhrich changed the title Using two scopes results in "Invalid cross-device link" Replacement results in "Invalid cross-device link" Oct 8, 2024
@igor47
Copy link

igor47 commented Oct 8, 2024

i'm having the same issue with a simple rename operation (srgn --python 'imports' app.core.celery_app app.celery_app). like the OP, i have the repo i'm working in on a different device/filesystem from my /tmp .

@igor47
Copy link

igor47 commented Oct 8, 2024

i believe the issue is due to the way the dependency tempfile works. here:

srgn/src/main.rs

Lines 541 to 543 in 9238fc1

let mut file = tempfile::Builder::new()
.prefix(env!("CARGO_PKG_NAME"))
.tempfile()?;

we declare file to be the result of tempfile::Builder (random question, i'm new to rust -- how the heck is tempfile even a valid symbol in this scope without a use tempfile at the top of the module?). this is probably created in the system temporary directory (/tmp on most linuxes). we then, after performing some replacement, call file.persist. the documentation for persist: https://docs.rs/tempfile/latest/tempfile/struct.NamedTempFile.html#method.persist

Note: Temporary files cannot be persisted across filesystems. Also neither the file contents nor the containing directory are synchronized, so the update may not yet have reached the disk when persist returns.

probably because persist tries to create a hardlink to the temp file at the persisted destination, and hardlinks do not work across filesystems. hard to tell from all the code, definitely a hard-link is in one of the paths...

probably, we should be using the tempfile_in argument to Builder and creating the temp files in a local temporary directory created at the start of the run to avoid potential cross-filesystem hardlink issues.

@alexpovel
Copy link
Owner

Thanks @lukasjuhrich and @igor47 for your detailed reports. I'll look into it more soon.

The tempfile with persist was just me trying to be smart/safe, but it's optional. It would be pretty easy to just use a more standard approach here, which fixes the issue. I'm mainly confused how I hadn't caught this myself, as I tested with a very vanilla Ubuntu machine where I thought /tmp as tmpfs is the default. I'll double check that.

As for the UX, agree! At the very least, the two positional arguments of [SCOPE] [REPLACEMENT] should be forced to follow any and all options and flags. Then they stick out more. That could be a quick fix.

Re: how tempfile is in scope: because the module path is fully qualified. use is not strictly required, just reads better often.

Re:

I wanted to find all the super() function calls inside methods without a certain decorator attached (@overload)

Yeah don't think that's possible! If at all, it'd require substantial regex use. The without part is not really supported, srgn doesn't have a concept of negation.

alexpovel added a commit that referenced this issue Oct 19, 2024
`persist` of `tempfile` does not work across
device boundaries, so if the device backing the
temporary directory is e.g. `tmpfs`, then
`persist` fails. It actually had no useful
benefit, so we can just remove it and write to the
destination directly.

Reproduced bug and confirmed fix on a Debian
machine with:

```console
$ sudo mkdir -p /path/to/temp_dir
$ sudo mount -t tmpfs -o size=100m tmpfs /path/to/temp_dir
$ sudo mount | rg 'temp'
tmpfs on /path/to/temp_dir type tmpfs (rw,relatime,size=102400k,inode64)
$ export TMPDIR=/path/to/temp_dir
$ cargo run -- --python class a whatever
```

`TMPDIR` env var is respected by `tempfile`. The
above `cargo run` fails without this patch. (There
is an end-to-end test for file writing, but that
has never failed for some reason... seems like
none of my machines (including Ubuntu 24) or CI
exhibited the issue).

Closes #145
@alexpovel
Copy link
Owner

Hey folks,

version 0.13.3 is released via #135 , which should fix this issue (the original one). #149 was created to track improvements to the UX, for which there's various options. More feedback, suggestions or even collaboration welcome!

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