trufflesuite / truffle

:warning: The Truffle Suite is being sunset. For information on ongoing support, migration options and FAQs, visit the Consensys blog. Thank you for all the support over the years.
https://consensys.io/blog/consensys-announces-the-sunset-of-truffle-and-ganache-and-new-hardhat?utm_source=github&utm_medium=referral&utm_campaign=2023_Sep_truffle-sunset-2023_announcement_
MIT License
14.02k stars 2.32k forks source link

`Profiler.updated` doesn't respect `contract_directory` option #4081

Open mmv08 opened 3 years ago

mmv08 commented 3 years ago

Issue

I'm trying to compile https://github.com/argentlabs/argent-contracts/tree/1eb980e922ecd092b99bad76d8fad02ed1bfc9a1 (the latest commit at the master branch at the time of writing). contracts_directory in the config option is set to contracts_directory: "contracts/wallet/", (https://github.com/argentlabs/argent-contracts/blob/1eb980e922ecd092b99bad76d8fad02ed1bfc9a1/truffle-config-wallet.js#L5) but it tries to compile contracts outside this directory which require different compiler version and thus fail.

Steps to Reproduce

  1. Clone https://github.com/argentlabs/argent-contracts/tree/1eb980e922ecd092b99bad76d8fad02ed1bfc9a1
  2. Run npm i
  3. Run npm run compile:wallet

Expected Behavior

The command runs successfully and contracts are compiled

Actual Results

The config includes contracts_directory: "./contracts/wallet/",, but it also tries to compile

> Compiling ./contracts/infrastructure/dapp/CurveFilter.sol
> Compiling ./lib_0.7/paraswap/V4/lib/curve/CurvePoolMock.sol

which are clearly outside of the directory and not imported. The same command on a mac with i7 works just fine.

Output:

~/p/p/argent-contracts master !1 ?1 ❯ npm run compile:wallet        6s 19:50:25

> argent-contracts@1.0.0 compile:wallet
> npx truffle compile --config truffle-config-wallet.js

> Warning: possible unsupported (undocumented in help) command line option: --config

Compiling your contracts...
===========================
> Compiling ./contracts/infrastructure/dapp/CurveFilter.sol
> Compiling ./contracts/wallet/BaseWallet.sol
> Compiling ./contracts/wallet/Proxy.sol
> Compiling ./lib_0.7/paraswap/V4/lib/curve/CurvePoolMock.sol

CompileError: ParserError: Source file requires different compiler version (current compiler is 0.8.3+commit.8d00100c.Emscripten.clang) - note that nightly builds are considered to be strictly less than the released version
 --> /Users/mmv/projects/personal/argent-contracts/lib_0.7/paraswap/V4/lib/curve/CurvePoolMock.sol:1:1:
  |
1 | pragma solidity 0.7.5;
  | ^^^^^^^^^^^^^^^^^^^^^^

Error: Truffle is currently using solc 0.8.3, but one or more of your contracts specify "pragma solidity 0.7.5".
Please update your truffle config or pragma statement(s).
(See https://trufflesuite.com/docs/truffle/reference/configuration#compiler-configuration for information on
configuring Truffle to use a specific solc compiler version.)

Compilation failed. See above.
    at run (/Users/mmv/projects/personal/argent-contracts/node_modules/truffle/build/webpack:/packages/compile-solidity/run.js:51:1)
    at Object.sourcesWithDependencies (/Users/mmv/projects/personal/argent-contracts/node_modules/truffle/build/webpack:/packages/compile-solidity/index.js:107:49)
    at necessary (/Users/mmv/projects/personal/argent-contracts/node_modules/truffle/build/webpack:/packages/compile-solidity/index.js:69:1)
    at /Users/mmv/projects/personal/argent-contracts/node_modules/truffle/build/webpack:/packages/workflow-compile/index.js:38:1
    at async Promise.all (index 0)
    at compile (/Users/mmv/projects/personal/argent-contracts/node_modules/truffle/build/webpack:/packages/workflow-compile/index.js:28:1)
    at Object.compile (/Users/mmv/projects/personal/argent-contracts/node_modules/truffle/build/webpack:/packages/workflow-compile/index.js:67:38)
Truffle v5.1.48 (core: 5.1.48)
Node v16.2.0

Environment

Misc

I ran node --inspect ./node_modules/.bin/truffle compile --config truffle-config-wallet.js and it all narrows to https://github.com/trufflesuite/truffle/blob/develop/packages/compile-common/src/profiler/profiler.ts#L24 return the wrong paths.

Screenshot 2021-05-30 at 19 37 01

paths = (4) ["/Users/mmv/projects/personal/argent-contracts/contracts/wallet/BaseWallet.sol", "/Users/mmv/projects/personal/argent-contracts/contracts/wallet/Proxy.sol", "/Users/mmv/projects/personal/argent-contracts/contracts/infrastructure/dapp/CurveFilter.sol", "/Users/mmv/projects/personal/argent-contracts/lib_0.7/paraswap/V4/lib/curve/CurvePoolMock.sol"],
fainashalts commented 3 years ago

Thanks for bringing this to our attention @mikheevm, we will definitely take a look as this is not expected behavior. In the meantime, it's worth deleting your build directory and trying again, just in case there's something going on there. We'll keep you posted!

@kevinbluer tagging you here for visibility in case you can test this with your M1 machine.

mmv08 commented 3 years ago

it's worth deleting your build directory and trying again, just in case there's something going on there

@fainashalts Thank you! Deleting my build directory worked, there were few contracts. I don't think it should try to recompile them if I set contracts_directory explicitly

cds-amal commented 3 years ago

Hi @mikheevm, if you have a public repo could you share the link, otherwise can you elaborate on what you're trying to achieve using contracts_directory. In the mean time we will try to test on an M1. cc: @kevinbluer

mmv08 commented 3 years ago

@cds-amal Hey 🙂 I left a repo link in the original issue. The issue is fixed by deleting the build folder. The problem is that it tried to recompile contracts that were already present there, even if they were not included in the path defined in the contracts_directory option

kevinbluer commented 3 years ago

hey @mikheevm, I've had a chance to pick this up on my M1, although interestingly so far I've not been able to repo. output from the most recent attempt...

➜  argent-contracts git:(1eb980e) npm run compile:wallet

> argent-contracts@1.0.0 compile:wallet
> npx truffle compile --config truffle-config-wallet.js

> Warning: possible unsupported (undocumented in help) command line option: --config

Compiling your contracts...
===========================
> Compiling ./contracts/modules/common/IModule.sol
> Compiling ./contracts/wallet/BaseWallet.sol
> Compiling ./contracts/wallet/IWallet.sol
> Compiling ./contracts/wallet/Proxy.sol
> Artifacts written to /Users/bluer/Developer/argent-contracts/build/contracts
> Compiled successfully using:
   - solc: 0.8.3+commit.8d00100c.Emscripten.clang

for reference I'm on a slightly newer build of truffle than the version you mentioned you'd tested against (e.g. 5.3.8). do you want to try updating on the off chance it resolves? in the interim, i'm going to do some more digging 🤔

Truffle v5.3.11 (core: 5.3.11)
Solidity - 0.8.3 (solc-js)
Node v16.4.0
Web3.js v1.3.6
mmv08 commented 3 years ago

@kevinbluer Thanks for taking a look. I tried upgrading it when I was looking for a fix and it didn't help. Please check my recent comment about what I think was the problem for me:

The issue is fixed by deleting the build folder. The problem is that it tried to recompile contracts that were already present there, even if they were not included in the path defined in the contracts_directory option

So, if you build contracts that use a different solidity compiler version, it will fail. I just tried to reproduce it again and here are the steps:

  1. npm run compile:infrastructure_0.5
  2. Change something in infrastructure 0.5 contracts. For example, go to argent-contracts/contracts/infrastructure_0.5/MultiSigWallet.sol and change line 25 to uint constant public MAX_OWNER_COUNT = 12
  3. npm run compile:wallet

Step 3 should fail because it would try to compile argent-contracts/contracts/infrastructure_0.5/MultiSigWallet.sol contract, but it's not included in contracts_directory option.

kevinbluer commented 3 years ago

ah gotcha...apols for missing that additional comment. that said, i just tried the above steps and it's still looking ok (at least as far is i can tell). output for reference in case i'm missing something, which is quite possible 😅

➜  argent-contracts git:(1eb980e) ✗ npm run compile:infrastructure_0.5

> argent-contracts@1.0.0 compile:infrastructure_0.5
> npx truffle compile --config truffle-config-infrastructure-0.5.js

> Warning: possible unsupported (undocumented in help) command line option: --config

Compiling your contracts...
===========================
> Compiling ./contracts/infrastructure/base/Owned.sol
> Compiling ./contracts/infrastructure/storage/IGuardianStorage.sol
> Compiling ./contracts/infrastructure/storage/ITransferStorage.sol
> Compiling ./contracts/infrastructure_0.5/ModuleRegistry.sol
> Compiling ./contracts/infrastructure_0.5/MultiSigWallet.sol
> Compiling ./contracts/infrastructure_0.5/storage/GuardianStorage.sol
> Compiling ./contracts/infrastructure_0.5/storage/Storage.sol
> Compiling ./contracts/infrastructure_0.5/storage/TransferStorage.sol
> Compiling ./contracts/wallet/IWallet.sol
> Compiling ./lib_0.5/other/ERC20.sol
> Artifacts written to /Users/bluer/Developer/argent-contracts/build/contracts
> Compiled successfully using:
   - solc: 0.5.4+commit.9549d8ff.Emscripten.clang

➜  argent-contracts git:(1eb980e) ✗ git status
HEAD detached at 1eb980e
Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
    modified:   contracts/infrastructure_0.5/MultiSigWallet.sol

no changes added to commit (use "git add" and/or "git commit -a")
➜  argent-contracts git:(1eb980e) ✗ npm run compile:wallet

> argent-contracts@1.0.0 compile:wallet
> npx truffle compile --config truffle-config-wallet.js

> Warning: possible unsupported (undocumented in help) command line option: --config

Compiling your contracts...
===========================
> Compiling ./contracts/wallet/BaseWallet.sol
> Compiling ./contracts/wallet/Proxy.sol
> Artifacts written to /Users/bluer/Developer/argent-contracts/build/contracts
> Compiled successfully using:
   - solc: 0.8.3+commit.8d00100c.Emscripten.clang
mmv08 commented 3 years ago

@kevinbluer That's weird, I have been able to reproduce it on two machines (i7 and m1) and the latest truffle version. Just to make sure, you did step 2 after step 1, right? You need to make a change in infrastructure 0.5 contracts after you compile it so truffle detects a change and tries to recompile it.

mmv08 commented 3 years ago

Here is a video of reproduction: https://www.loom.com/share/f69e9b6040214c43bd66405750dc9e57

kevinbluer commented 3 years ago

hey @mikheevm, good news in that i've been able to repro it! super odd though, as i tried running the commands in vscode's built-in terminal so as to closely mirror that of your environment (per the video) and it barfed. same again when i subsequently ran it in an external terminal (e.g. iterm2) though so it's quite possible that i did mess up the order...although i was pretty careful so 🤷

will follow-up again shortly 👌

kevinbluer commented 3 years ago

alrighty, looks like i might have an interim fix...can you try changing the solc version in both truffle-config-wallet.js and truffle-config-infrastructure-0.5.js to "pragma"...e.g.

version: "pragma"

more detail on specifying pragma as the version here

gnidan commented 3 years ago

The issue is fixed by deleting the build folder. The problem is that it tried to recompile contracts that were already present there, even if they were not included in the path defined in the contracts_directory option

Now this sounds like something to go on... we'll continue to see what we can figure out

eggplantzzz commented 3 years ago

Just to check back in on this, I can't seem to recreate this. Can you confirm for me that this is still an issue?

mmv08 commented 3 years ago

@eggplantzzz reproduce where? on the latest version or the env info I provided?

mmv08 commented 3 years ago

I can confirm it's reproducible with truffle 5.4.3. I used steps from https://github.com/trufflesuite/truffle/issues/4081#issuecomment-875958740 and the video I recorded

cds-amal commented 3 years ago

@kevinbluer - Maybe unrelated, but I'm not sure about the behavior of npx in the package.json scripts. Would it use the same truffle version on different machines and different times?

Since truffle is installed as a dependency then it should be accessible as truffle ... in scripts without the need for npx.

mmv08 commented 3 years ago

@cds-amal npx checks if the dependency exists locally first

cds-amal commented 3 years ago

Thanks @mikheevm, the documentation indicates locally means: first check $PATH then local project binaries, which could lead to different behavior depending on a user's $PATH env.

By default, npx will check whether exists in $PATH, or in the local project binaries, and execute that. If is not found, it will be installed prior to execution.

mmv08 commented 3 years ago

who puts truffle to path?

mmv08 commented 3 years ago

I can reproduce it without npx

kevinbluer commented 3 years ago

hey @mikheevm we've just been digging in to this one and can verify it isn't an m1 specific issue (e.g. we've been able to repo on other architectures). beyond this, we should have a bigger update shortly 👍

gnidan commented 3 years ago

@mikheevm did you try the version: "pragma" suggestion that @kevinbluer mentioned above?

mmv08 commented 3 years ago

@gnidan i just deleted already compiled contracts from build folder

eggplantzzz commented 3 years ago

So the problem is actually the profiler. When compiling, Truffle looks at all the artifacts in the build directory and compares them to their sources to see if anything has changed. Because the artifacts folder is full of builds from ALL contracts that have been compiled, it picks up the contract that was changed in your reproduction steps. Even though it is from the contracts directory you did not specify, it will attempt to compile it with the contracts that require a different version of the compiler and fail.

@mikheevm You might check out the pragma feature which most likely takes longer but allows you to use multiple versions of the Solidity compiler in one compile. You can set version to "pragma" in your config and then attempt compilation of all the contracts.

mmv08 commented 3 years ago

I thought we figured it out there: https://github.com/trufflesuite/truffle/issues/4081#issuecomment-858081641

As I said before I have already overcome the issue, it's up to you if you want to fix it. In my opinion, this is an unobvious behavior and should be fixed (if it doesn't require rewriting the whole truffle ofc)

eggplantzzz commented 3 years ago

@mikheevm Yes, we need to eventually fix this but it is a non-trivial problem at the moment. And I'm glad you got this resolved for your situation, I was just trying to offer you another option that you might feel is useful. Thanks again for reporting this!

haltman-at commented 2 years ago

Is this related to #2724 at all? (Sorry, don't have time to check right now, so just asking.)

eggplantzzz commented 2 years ago

No @haltman-at, it is something different.