ChainMovers / suibase

Sui development environment and cookbook.
https://suibase.io
Apache License 2.0
35 stars 6 forks source link

Why does tsui client publish not work? #65

Closed georgescharlesbrain closed 10 months ago

georgescharlesbrain commented 1 year ago

"tsui client publish --gas-budget 100000000" Unable to find package manifest in '/home/xxx/suibase/workdirs/testnet/logs/sui.log' or in its parents

Does publishing via the client not work anymore, and are you obliged to use "testnet publish"?

mario4tier commented 1 year ago

This is a bug in suibase. "tsui" should work as "sui".

Workaround Specify on your command line the [package path] instead of defaulting to the current directory:

Like "tsui client publish --gas-budget 100000000 /home/user/myproject/"

Usage: tsui client publish [OPTIONS] --gas-budget [package_path]

What is the bug? A while ago the sui client was polluting the filesystem with a bunch of sui.log written by default in the current directory, so a trick was for the suibase script to change the current directory to "logs/sui.log" before calling the sui client.

That cause trouble with commands that rely on using the current directory (there is another case with a keytool command forcing its output file into the current directory).

Now the sui client behave better for its sui.log... so may be that trick of changing the current directory should be removed.

georgescharlesbrain commented 10 months ago

Encountered this issue again. "lsui move test" also gives the error:
Unable to find package manifest in '~/suibase/workdirs/localnet/logs/sui.log' or in its parents It's a bit cumbersome having to add -p to every command IMO.

mario4tier commented 10 months ago

Summary

Thanks for reporting this.

The fix for 'lsui move' done.

Work-in-progress for 'lsui client publish' (more complicated than expected).

Implementation Details

Correction on 'What is the bug?' They changed back and forth the fix to sui.log being noisy so I am NOT removing yet the "trick" of Suibase changing the current directory. Reference: https://github.com/MystenLabs/sui/commit/49657dd789290a54aab206e3ee8e227449ae12e2#comments

The fixes instead are to compensate (by adding options to current directory at the time the script was started) for when the user does not specify a path.

Why was the fix done for 'lsui move' quickly but not done for 'lsui client'? 'move' was easy because they provide unambiguous --path and --install-dir options.

There are no such options for 'lsui client publish' requiring significantly more complicated command line parsing to find if the user did specify a path or not (which can be inserted anywhere among the options, can't assume always at the end of the command line).

mario4tier commented 10 months ago

All fix now commited.

Did fix also scenario where the user specify a relative path with "." or ".." (e.g. 'lsui move test -p ./some_directory'). The relative path will now be properly interpreted to be from the current dir.

Do not hesitate to re-open if finding new cases. Thanks.