Bugfix/re adding files

[dstadelm]

Hi, this changes make the handling of adding libraries and sourcefiles less restrictive. Adding of these multiple times results in ignoring the request rather than throwing an exception.

The diff shows mostly white space changes due to the python indentation

[kraigher]

What I do not like about this is that it allows adding two files with the same name but different contents without any error or warning. Such behaviour can be ok in an user wrapper layer but not in the tool itself.

[dstadelm]

I don't understand. My assumption is that this should only have an impact if a file (the same physical file on disk) is added once again. Is my assumption therefore wrong, that the file_name which is the key for the _source_files dictionary in library is a fully qualified path name? If so, I will have to find a different way to discern if the file 'exactly the same file' is already in the library.

Furthermore, it is totally valid vhdl to write to files with the same name, but different content. This might be very ugly and should be avoided by all means, but sometimes one has to integrate code of third parties and one does not have a choice :-/

[kraigher]

The file name key is the full absolute path. The problem occurs when the contents of the file is changed between the two additions which can happen when files are carelessly programmatically created.

[kraigher]

I can see three ways to resolve this:

1. Do nothing. It is already possible for the user to create this behavior in the client code.

2. Make the behavior opt-in on the Public API.

3. Check the hash sum of files added twice and only complain if they differ.

[dstadelm]

I was thinking along the same lines. I'll implement a version using the hash, would that work for you?

[dstadelm]

Hi @kraigher @LarsAsplund
I've just looked at the code once more, and checked what you are using as a key for the <_source_files> dict in class library. It seems you are using the absolute path of a file. I mean /home/user/project/file.vhd will always have the same content as /home/user/project/file.vhd I don't really think one requires a hash to verify that. Am I missing something?

In project.py:add_source_file I've added following print line

             print("FILE NAME = " + file_name, file=sys.stderr)
             source_file = library.get_source_file(file_name)

This generates following output

FILE NAME = /opt/vunit/vunit/vhdl/logging/src/logger_pkg-body.vhd
FILE NAME = /opt/vunit/vunit/vhdl/logging/src/log_handler_pkg-body.vhd
FILE NAME = /opt/vunit/vunit/vhdl/logging/src/log_handler_pkg.vhd
FILE NAME = /opt/vunit/vunit/vhdl/logging/src/log_levels_pkg-body.vhd
FILE NAME = /opt/vunit/vunit/vhdl/logging/src/log_levels_pkg.vhd
FILE NAME = /opt/vunit/vunit/vhdl/logging/src/logger_pkg.vhd
FILE NAME = /opt/vunit/vunit/vhdl/logging/src/print_pkg-body.vhd
FILE NAME = /opt/vunit/vunit/vhdl/logging/src/log_deprecated_pkg.vhd
FILE NAME = /opt/vunit/vunit/vhdl/logging/src/ansi_pkg.vhd
FILE NAME = /opt/vunit/vunit/vhdl/logging/src/print_pkg.vhd
FILE NAME = /opt/vunit/vunit/vhdl/logging/src/file_pkg.vhd
FILE NAME = /opt/vunit/vunit/vhdl/string_ops/src/string_ops.vhd
FILE NAME = /opt/vunit/vunit/vhdl/check/src/check_deprecated_pkg.vhd
FILE NAME = /opt/vunit/vunit/vhdl/check/src/check_api.vhd
FILE NAME = /opt/vunit/vunit/vhdl/check/src/check.vhd
FILE NAME = /opt/vunit/vunit/vhdl/check/src/checker_pkg.vhd
FILE NAME = /opt/vunit/vunit/vhdl/check/src/checker_pkg-body.vhd
FILE NAME = /opt/vunit/vunit/vhdl/dictionary/src/dictionary.vhd
FILE NAME = /opt/vunit/vunit/vhdl/run/src/runner_pkg.vhd
FILE NAME = /opt/vunit/vunit/vhdl/run/src/run_deprecated_pkg.vhd
FILE NAME = /opt/vunit/vunit/vhdl/run/src/run_api.vhd
FILE NAME = /opt/vunit/vunit/vhdl/run/src/run_types.vhd
FILE NAME = /opt/vunit/vunit/vhdl/run/src/run.vhd
FILE NAME = /opt/vunit/vunit/vhdl/path/src/path.vhd
FILE NAME = /project/fw/lib/lib-scs/sim/tb/tb_fifo_in.vhd
FILE NAME = /project/fw/lib/lib-scs/sim/tb/tb_func_pkg.vhd
FILE NAME = /project/fw/lib/lib-scs/sim/tb/tb_fifo_out.vhd
FILE NAME = /project/fw/lib/lib-scs/synth/memory/fifos/fifo/vhdl/fifo.vhd
FILE NAME = /project/fw/lib/lib-scs/sim/util/text/txt_util.vhd

So I'm confident my suggested solution should work just fine.

[kraigher]

The problem occurs when the contents of the file is changed between the two additions. It can happen when files are carelessly programmatically created between two calls to add_source_files.

[dstadelm]

Oh, I see! Hash it is then.
Thx for the input!

[kraigher]

You can use the file_content_hash funciton which is already used in project.py to compute a hash.

[dstadelm]

Funny, when I test adding a library 'lib' and 'Lib' the KeyError is thrown.

[kraigher]

Could you be more specific and provide details?

[dstadelm]

There is a testcase failing:
TestProject.test_error_on_case_insensitive_library_name_conflict
This tests that one can not add library lib and Lib, as vhdl is case insensitive. An KeyError Exception is expected. I've tested this in our environment, and it worked, I do not understand why the test fails.

I tried to run the tests locally, but somehow the setup for running the tests seems to be a little more complex than just installing pytest and pylint. I'm a python noob, so I'm sorry for the inconvenience I'm possibly creating.

[dstadelm]

I've found out what went wrong. Fix is on the way.

[dstadelm]

@kraigher There are some linting errors and apt-get stuff that fail in the ci, they do not have anything to do with me, do they? The library testcase now runs, the only test failing is the test concerning adding a file twice, which of course is allowed now. Are there any further actions required from my side?

[kraigher]

I am quite busy now but I will have a look at this during this weekend or next week.

[dstadelm]

No worries, I appreciate your help. I can only imagine what a lot of work this is next to the daily regular work you do. I myself implemented a compile order generator for vhdl, verilog and system verilog which we have been using the last ten years. Though the tool could only create the order and invoke compilation and therefore covered less topics than vunit I know what it is like to have a side project, on which many colleagues rely, next to regular work.
I hope the success of vunit continues, as this is the createst reward developers can get.

[kraigher]

I added this feature now with a different implementation. It is possible to add duplicate files with identical content without error. There is a warning log message though.

[krishnan-gopal]

Hi. I am trying out this feature. It looks like the allow_replacement argument in the add_library() function is only applied at the project class, and not the ui class. The add_library() in the ui class calls the function with the same name in the project class. Can this be fixed ?
Or am I doing something wrong ?

[kraigher]

@krishnan-gopal This issue/feature was related to adding duplicate files not duplicate libraries. The allow_replacement flag in the Project is just dead code and should be removed. After implementing this issue it is now possible to add the same file path twice if it has identical contents in both occations.